[1/3] discover/platform-powerpc: read bootdev config from IPMI boot mailbox
diff mbox series

Message ID 20181203030035.26487-1-sam@mendozajonas.com
State Superseded
Headers show
Series
  • [1/3] discover/platform-powerpc: read bootdev config from IPMI boot mailbox
Related show

Commit Message

Samuel Mendoza-Jonas Dec. 3, 2018, 3 a.m. UTC
The IPMI Get System Boot Options commands includes parameter 7, the
"boot initiator mailbox". This can be used to hold arbitrary data to
influence the boot order.

Use this to provide an alternate bootdev configuration to Petitboot that
will override the one saved to NVRAM. This provides more fine grained
override options than the existing device-type based overrides.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 discover/platform-powerpc.c | 230 +++++++++++++++++++++++++++++++++++-
 discover/platform.c         |   2 +-
 discover/platform.h         |   2 +
 3 files changed, 232 insertions(+), 2 deletions(-)

Comments

Samuel Mendoza-Jonas Dec. 6, 2018, 12:30 a.m. UTC | #1
On Mon, 2018-12-03 at 14:00 +1100, Samuel Mendoza-Jonas wrote:
> The IPMI Get System Boot Options commands includes parameter 7, the
> "boot initiator mailbox". This can be used to hold arbitrary data to
> influence the boot order.
> 
> Use this to provide an alternate bootdev configuration to Petitboot that
> will override the one saved to NVRAM. This provides more fine grained
> override options than the existing device-type based overrides.

It was pointed out this was a little short of details of the
implementation which is pretty fair in hindsight. I may include this in
the actual commit message as well but for now here's a quick run down:

Petitboot already takes advantage of the Get/Set System Boot Option
commands to recognise "boot overrides" that have been set. Specifically
this uses parameter 5, "boot flags", of the command as described by Table
28, "Boot Option Parameters" in the IPMI spec. These boot flags are
limited to generic options such as "boot from disk", "boot from network",
etc.

What this series does it take advantage of parameter 7, "boot initiator
mailbox", in the same command. The difference here is that this is just a
bunch of space we can put something into. In particular we're using it to
hold a "petitboot,bootdevs=..." parameter the same as we would in NVRAM.

The content of parameter 7 is
	1 byte: block number
	2-16 bytes: block data

The BMC is required to hold *at least* 80 bytes of data, which means 5
blocks (16 bytes * 5 = 80 bytes).

The BMC is also required to set the first 3 bytes of the first block to
an IANA Enterprise ID Number. This series recognises the IBM number (2)
so the first three bytes must be 2, 0, 0 (least-significant first).

So to store the string "petitboot,bootdevs="net usb disk" the format of
the blocks would be:

0:	|2|0|0|p|e|t|i|t|b|o|o|t|,|b|o|o|
1:	|t|d|e|v|s|=|n|e|t| |u|s|b| |d|i|
2:	|s|k| | | | | | | | | | | | | | |

The string must be null-terminated but the remaining buffer contents do
not necessarily need to be set.
Petitboot will request up to 16 blocks[0] and then use this string if it
is present in order to set the boot order. I would *highly* recommend a
BMC supports well over 5 blocks so that we can fit disk UUIDS and LV
names.

Each block is selected individually, eg. Petitboot will send several
requests like,
	get-sys-boot-option, parameter 7, block 0
	get-sys-boot-option, parameter 7, block 1
	get-sys-boot-option, parameter 7, block 2
	...

And the BMC must return the corresponding block. Petitboot will similarly
send set-sys-boot-option commands to set the buffer (to clear it for
example).

The script in patch two says out the raw format for get & set to use with
ipmitool, and basically shows what the BMC would need to expect and how
to respond.

These details can be found in section 28.13 in v2 rev1.1 of the IPMI
spec, and page 398 specifically for the mailbox.

[0] Re-reading the spec we could technically read up to 255 blocks.. hmm!
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> ---
>  discover/platform-powerpc.c | 230 +++++++++++++++++++++++++++++++++++-
>  discover/platform.c         |   2 +-
>  discover/platform.h         |   2 +
>  3 files changed, 232 insertions(+), 2 deletions(-)
> 
> diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
> index f8f33054..c674e17c 100644
> --- a/discover/platform-powerpc.c
> +++ b/discover/platform-powerpc.c
> @@ -27,13 +27,18 @@ static const char *devtree_dir = "/proc/device-tree/";
>  struct platform_powerpc {
>  	struct param_list *params;
>  	struct ipmi	*ipmi;
> -	bool		ipmi_bootdev_persistent;
> +	char		*ipmi_mailbox_original_config;
>  	int		(*get_ipmi_bootdev)(
>  				struct platform_powerpc *platform,
>  				uint8_t *bootdev, bool *persistent);
>  	int		(*clear_ipmi_bootdev)(
>  				struct platform_powerpc *platform,
>  				bool persistent);
> +	int		(*get_ipmi_boot_mailbox)(
> +				struct platform_powerpc *platform,
> +				char **buf);
> +	int		(*clear_ipmi_boot_mailbox)(
> +				struct platform_powerpc *platform);
>  	int 		(*set_os_boot_sensor)(
>  				struct platform_powerpc *platform);
>  	void		(*get_platform_versions)(struct system_info *info);
> @@ -429,6 +434,184 @@ static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
>  	return 0;
>  }
>  
> +static int get_ipmi_boot_mailbox_block(struct platform_powerpc *platform,
> +		char *buf, uint8_t block)
> +{
> +	const size_t blocksize = 16;
> +	uint8_t resp[3 + blocksize];
> +	uint16_t resp_len;
> +	char *debug_buf;
> +	int rc;
> +	uint8_t req[] = {
> +		0x07,  /* parameter selector: boot initiator mailbox */
> +		block, /* set selector */
> +		0x00,  /* no block selector */
> +	};
> +
> +	resp_len = sizeof(resp);
> +	rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_CHASSIS,
> +			IPMI_CMD_CHASSIS_GET_SYSTEM_BOOT_OPTIONS,
> +			req, sizeof(req),
> +			resp, &resp_len,
> +			ipmi_timeout);
> +	if (rc) {
> +		pb_log("platform: error reading IPMI boot options\n");
> +		return -1;
> +	}
> +
> +	if (resp_len < sizeof(resp)) {
> +		pb_log("platform: unexpected length (%d) in "
> +				"boot options mailbox response\n", resp_len);
> +		return -1;
> +	}
> +
> +	debug_buf = format_buffer(platform, resp, resp_len);
> +	pb_debug_fn("IPMI bootdev mailbox block %hu:\n%s\n", block, debug_buf);
> +	talloc_free(debug_buf);
> +
> +	if (resp[0] != 0) {
> +		pb_log("platform: non-zero completion code %d from IPMI req\n",
> +				resp[0]);
> +		return -1;
> +	}
> +
> +	/* check for correct parameter version */
> +	if ((resp[1] & 0xf) != 0x1) {
> +		pb_log("platform: unexpected version (0x%x) in "
> +				"boot mailbox response\n", resp[0]);
> +		return -1;
> +	}
> +
> +	/* check for valid paramters */
> +	if (resp[2] & 0x80) {
> +		pb_debug("platform: boot mailbox parameters are invalid/locked\n");
> +		return -1;
> +	}
> +
> +	memcpy(buf, &resp[3], blocksize);
> +
> +	return 0;
> +}
> +
> +static int get_ipmi_boot_mailbox(struct platform_powerpc *platform,
> +		char **buf)
> +{
> +	const size_t blocksize = 16;
> +	char *mailbox_buffer, *prefix;
> +	int content_size;
> +	uint8_t i;
> +	int rc;
> +
> +	mailbox_buffer = talloc_array(platform, char, blocksize * 16);
> +	if (!mailbox_buffer) {
> +		pb_log_fn("Failed to allocate mailbox buffer\n");
> +		return -1;
> +	}
> +
> +	/*
> +	 * The mailbox can hold up to 16 16-byte blocks which we must request
> +	 * individually. The mininum number required by the spec is 5 blocks,
> +	 * but work with what we've got if we get less.
> +	 */
> +	for (i = 0; i < 16; i++) {
> +		rc = get_ipmi_boot_mailbox_block(platform,
> +				mailbox_buffer + (i * 16), i);
> +		if (rc && i == 0) {
> +			/* Immediate failure, no blocks read */
> +			return -1;
> +		}
> +		if (rc)
> +			break;
> +
> +		if (i == 0) {
> +			/*
> +			 * The first three bytes of block zero are an IANA
> +			 * Enterprise ID number. Check it matches the IBM
> +			 * number, '2'.
> +			 */
> +			if (mailbox_buffer[0] != 0x02 ||
> +				mailbox_buffer[1] != 0x00 ||
> +				mailbox_buffer[2] != 0x00) {
> +				pb_log_fn("IANA number unrecognised: 0x%x:0x%x:0x%x\n",
> +						mailbox_buffer[0],
> +						mailbox_buffer[1],
> +						mailbox_buffer[2]);
> +				return -1;
> +			}
> +		}
> +	}
> +
> +	if (i < 5)
> +		pb_log_fn("Only %hu blocks read, spec requires at least 5.\n"
> +			  "Send a bug report to your preferred BMC vendor!\n",
> +			  i);
> +	else
> +		pb_debug_fn("%hu blocks read\n", i);
> +
> +	prefix = talloc_strndup(mailbox_buffer, mailbox_buffer + 3,
> +			strlen("petitboot,bootdevs="));
> +	if (!prefix) {
> +		pb_log_fn("Couldn't check prefix\n");
> +		talloc_free(mailbox_buffer);
> +		return -1;
> +	}
> +
> +	if (strncmp(prefix, "petitboot,bootdevs=",
> +				strlen("petitboot,bootdevs=")) != 0 ) {
> +		/* Empty or garbage */
> +		pb_debug_fn("Buffer looks unconfigured\n");
> +		talloc_free(mailbox_buffer);
> +		*buf = NULL;
> +		return 0;
> +	}
> +
> +
> +	/* Don't include IANA number in buffer */
> +	content_size = (i + 1) * blocksize - 3 - strlen("petitboot,bootdevs=");
> +	*buf = talloc_memdup(platform,
> +			mailbox_buffer + 3 + strlen("petitboot,bootdevs="),
> +			content_size + 1);
> +	(*buf)[content_size] = '\0';
> +
> +	talloc_free(mailbox_buffer);
> +	return 0;
> +}
> +
> +static int clear_ipmi_boot_mailbox(struct platform_powerpc *platform)
> +{
> +	uint8_t req[18] = {0}; /* req (2) + blocksize (16) */
> +	uint16_t resp_len;
> +	uint8_t resp[1];
> +	uint8_t i;
> +	int rc;
> +
> +	req[0] = 0x07;  /* parameter selector: boot initiator mailbox */
> +
> +	resp_len = sizeof(resp);
> +
> +	for (i = 0; i < 16; i++) {
> +		req[1] = i; /* set selector */
> +		rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_CHASSIS,
> +				IPMI_CMD_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> +				req, sizeof(req),
> +				resp, &resp_len,
> +				ipmi_timeout);
> +
> +		if (rc || resp[0]) {
> +			if (i == 0) {
> +				pb_log("platform: error clearing IPMI boot mailbox, rc %d resp[0] %hu\n",
> +						rc, resp[0]);
> +				return -1;
> +			} else
> +				break;
> +		}
> +	}
> +
> +	pb_debug_fn("Cleared %hu blocks\n", i);
> +
> +	return 0;
> +}
> +
>  static int set_ipmi_os_boot_sensor(struct platform_powerpc *platform)
>  {
>  	int sensor_number;
> @@ -605,6 +788,31 @@ static int load_config(struct platform *p, struct config *config)
>  	if (rc)
>  		pb_log_fn("Failed to parse nvram\n");
>  
> +	/*
> +	 * If we have an IPMI mailbox configuration available use it instead of
> +	 * the boot order found in NVRAM.
> +	 */
> +	if (platform->get_ipmi_boot_mailbox) {
> +		char *mailbox;
> +		struct param *param;
> +		rc = platform->get_ipmi_boot_mailbox(platform, &mailbox);
> +		if (!rc && mailbox) {
> +			platform->ipmi_mailbox_original_config =
> +				talloc_strdup(
> +					platform,
> +					param_list_get_value(
> +						platform->params, "petitboot,bootdevs"));
> +			param_list_set(platform->params, "petitboot,bootdevs",
> +					mailbox, false);
> +			param = param_list_get_param(platform->params,
> +					"petitboot,bootdevs");
> +			/* Avoid writing this to NVRAM */
> +			param->modified = false;
> +			config->ipmi_bootdev_mailbox = true;
> +			talloc_free(mailbox);
> +		}
> +	}
> +
>  	config_populate_all(config, platform->params);
>  
>  	if (platform->get_ipmi_bootdev) {
> @@ -630,6 +838,7 @@ static int save_config(struct platform *p, struct config *config)
>  {
>  	struct platform_powerpc *platform = to_platform_powerpc(p);
>  	struct config *defaults;
> +	struct param *param;
>  
>  	if (config->ipmi_bootdev == IPMI_BOOTDEV_INVALID &&
>  	    platform->clear_ipmi_bootdev) {
> @@ -639,6 +848,23 @@ static int save_config(struct platform *p, struct config *config)
>  		config->ipmi_bootdev_persistent = false;
>  	}
>  
> +	if (!config->ipmi_bootdev_mailbox &&
> +			platform->ipmi_mailbox_original_config) {
> +		param = param_list_get_param(platform->params,
> +				"petitboot,bootdevs");
> +		/* Restore old boot order if unmodified */
> +		if (!param->modified) {
> +			param_list_set(platform->params, "petitboot,bootdevs",
> +					platform->ipmi_mailbox_original_config,
> +					false);
> +			param->modified = false;
> +			config_populate_bootdev(config, platform->params);
> +		}
> +		platform->clear_ipmi_boot_mailbox(platform);
> +		talloc_free(platform->ipmi_mailbox_original_config);
> +		platform->ipmi_mailbox_original_config = NULL;
> +	}
> +
>  	defaults = talloc_zero(platform, struct config);
>  	config_set_defaults(defaults);
>  
> @@ -718,6 +944,8 @@ static bool probe(struct platform *p, void *ctx)
>  		platform->ipmi = ipmi_open(platform);
>  		platform->get_ipmi_bootdev = get_ipmi_bootdev_ipmi;
>  		platform->clear_ipmi_bootdev = clear_ipmi_bootdev_ipmi;
> +		platform->get_ipmi_boot_mailbox = get_ipmi_boot_mailbox;
> +		platform->clear_ipmi_boot_mailbox = clear_ipmi_boot_mailbox;
>  		platform->set_os_boot_sensor = set_ipmi_os_boot_sensor;
>  	} else if (!stat(sysparams_dir, &statbuf)) {
>  		pb_debug("platform: using sysparams for IPMI paramters\n");
> diff --git a/discover/platform.c b/discover/platform.c
> index 237da3a9..8a23afd3 100644
> --- a/discover/platform.c
> +++ b/discover/platform.c
> @@ -455,7 +455,7 @@ static int read_bootdev(void *ctx, char **pos, struct autoboot_option *opt)
>  	return rc;
>  }
>  
> -static void config_populate_bootdev(struct config *config,
> +void config_populate_bootdev(struct config *config,
>  	const struct param_list *pl)
>  {
>  	struct autoboot_option *opt, *new = NULL;
> diff --git a/discover/platform.h b/discover/platform.h
> index 29405626..1ac905af 100644
> --- a/discover/platform.h
> +++ b/discover/platform.h
> @@ -27,6 +27,8 @@ const struct config *config_get(void);
>  int config_set(struct config *config);
>  void config_set_defaults(struct config *config);
>  void config_set_autoboot(bool autoboot_enabled);
> +void config_populate_bootdev(struct config *config,
> +	const struct param_list *pl);
>  void config_populate_all(struct config *config, const struct param_list *pl);
>  
>  void params_update_network_values(struct param_list *pl,

Patch
diff mbox series

diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
index f8f33054..c674e17c 100644
--- a/discover/platform-powerpc.c
+++ b/discover/platform-powerpc.c
@@ -27,13 +27,18 @@  static const char *devtree_dir = "/proc/device-tree/";
 struct platform_powerpc {
 	struct param_list *params;
 	struct ipmi	*ipmi;
-	bool		ipmi_bootdev_persistent;
+	char		*ipmi_mailbox_original_config;
 	int		(*get_ipmi_bootdev)(
 				struct platform_powerpc *platform,
 				uint8_t *bootdev, bool *persistent);
 	int		(*clear_ipmi_bootdev)(
 				struct platform_powerpc *platform,
 				bool persistent);
+	int		(*get_ipmi_boot_mailbox)(
+				struct platform_powerpc *platform,
+				char **buf);
+	int		(*clear_ipmi_boot_mailbox)(
+				struct platform_powerpc *platform);
 	int 		(*set_os_boot_sensor)(
 				struct platform_powerpc *platform);
 	void		(*get_platform_versions)(struct system_info *info);
@@ -429,6 +434,184 @@  static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
 	return 0;
 }
 
+static int get_ipmi_boot_mailbox_block(struct platform_powerpc *platform,
+		char *buf, uint8_t block)
+{
+	const size_t blocksize = 16;
+	uint8_t resp[3 + blocksize];
+	uint16_t resp_len;
+	char *debug_buf;
+	int rc;
+	uint8_t req[] = {
+		0x07,  /* parameter selector: boot initiator mailbox */
+		block, /* set selector */
+		0x00,  /* no block selector */
+	};
+
+	resp_len = sizeof(resp);
+	rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_CHASSIS,
+			IPMI_CMD_CHASSIS_GET_SYSTEM_BOOT_OPTIONS,
+			req, sizeof(req),
+			resp, &resp_len,
+			ipmi_timeout);
+	if (rc) {
+		pb_log("platform: error reading IPMI boot options\n");
+		return -1;
+	}
+
+	if (resp_len < sizeof(resp)) {
+		pb_log("platform: unexpected length (%d) in "
+				"boot options mailbox response\n", resp_len);
+		return -1;
+	}
+
+	debug_buf = format_buffer(platform, resp, resp_len);
+	pb_debug_fn("IPMI bootdev mailbox block %hu:\n%s\n", block, debug_buf);
+	talloc_free(debug_buf);
+
+	if (resp[0] != 0) {
+		pb_log("platform: non-zero completion code %d from IPMI req\n",
+				resp[0]);
+		return -1;
+	}
+
+	/* check for correct parameter version */
+	if ((resp[1] & 0xf) != 0x1) {
+		pb_log("platform: unexpected version (0x%x) in "
+				"boot mailbox response\n", resp[0]);
+		return -1;
+	}
+
+	/* check for valid paramters */
+	if (resp[2] & 0x80) {
+		pb_debug("platform: boot mailbox parameters are invalid/locked\n");
+		return -1;
+	}
+
+	memcpy(buf, &resp[3], blocksize);
+
+	return 0;
+}
+
+static int get_ipmi_boot_mailbox(struct platform_powerpc *platform,
+		char **buf)
+{
+	const size_t blocksize = 16;
+	char *mailbox_buffer, *prefix;
+	int content_size;
+	uint8_t i;
+	int rc;
+
+	mailbox_buffer = talloc_array(platform, char, blocksize * 16);
+	if (!mailbox_buffer) {
+		pb_log_fn("Failed to allocate mailbox buffer\n");
+		return -1;
+	}
+
+	/*
+	 * The mailbox can hold up to 16 16-byte blocks which we must request
+	 * individually. The mininum number required by the spec is 5 blocks,
+	 * but work with what we've got if we get less.
+	 */
+	for (i = 0; i < 16; i++) {
+		rc = get_ipmi_boot_mailbox_block(platform,
+				mailbox_buffer + (i * 16), i);
+		if (rc && i == 0) {
+			/* Immediate failure, no blocks read */
+			return -1;
+		}
+		if (rc)
+			break;
+
+		if (i == 0) {
+			/*
+			 * The first three bytes of block zero are an IANA
+			 * Enterprise ID number. Check it matches the IBM
+			 * number, '2'.
+			 */
+			if (mailbox_buffer[0] != 0x02 ||
+				mailbox_buffer[1] != 0x00 ||
+				mailbox_buffer[2] != 0x00) {
+				pb_log_fn("IANA number unrecognised: 0x%x:0x%x:0x%x\n",
+						mailbox_buffer[0],
+						mailbox_buffer[1],
+						mailbox_buffer[2]);
+				return -1;
+			}
+		}
+	}
+
+	if (i < 5)
+		pb_log_fn("Only %hu blocks read, spec requires at least 5.\n"
+			  "Send a bug report to your preferred BMC vendor!\n",
+			  i);
+	else
+		pb_debug_fn("%hu blocks read\n", i);
+
+	prefix = talloc_strndup(mailbox_buffer, mailbox_buffer + 3,
+			strlen("petitboot,bootdevs="));
+	if (!prefix) {
+		pb_log_fn("Couldn't check prefix\n");
+		talloc_free(mailbox_buffer);
+		return -1;
+	}
+
+	if (strncmp(prefix, "petitboot,bootdevs=",
+				strlen("petitboot,bootdevs=")) != 0 ) {
+		/* Empty or garbage */
+		pb_debug_fn("Buffer looks unconfigured\n");
+		talloc_free(mailbox_buffer);
+		*buf = NULL;
+		return 0;
+	}
+
+
+	/* Don't include IANA number in buffer */
+	content_size = (i + 1) * blocksize - 3 - strlen("petitboot,bootdevs=");
+	*buf = talloc_memdup(platform,
+			mailbox_buffer + 3 + strlen("petitboot,bootdevs="),
+			content_size + 1);
+	(*buf)[content_size] = '\0';
+
+	talloc_free(mailbox_buffer);
+	return 0;
+}
+
+static int clear_ipmi_boot_mailbox(struct platform_powerpc *platform)
+{
+	uint8_t req[18] = {0}; /* req (2) + blocksize (16) */
+	uint16_t resp_len;
+	uint8_t resp[1];
+	uint8_t i;
+	int rc;
+
+	req[0] = 0x07;  /* parameter selector: boot initiator mailbox */
+
+	resp_len = sizeof(resp);
+
+	for (i = 0; i < 16; i++) {
+		req[1] = i; /* set selector */
+		rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_CHASSIS,
+				IPMI_CMD_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
+				req, sizeof(req),
+				resp, &resp_len,
+				ipmi_timeout);
+
+		if (rc || resp[0]) {
+			if (i == 0) {
+				pb_log("platform: error clearing IPMI boot mailbox, rc %d resp[0] %hu\n",
+						rc, resp[0]);
+				return -1;
+			} else
+				break;
+		}
+	}
+
+	pb_debug_fn("Cleared %hu blocks\n", i);
+
+	return 0;
+}
+
 static int set_ipmi_os_boot_sensor(struct platform_powerpc *platform)
 {
 	int sensor_number;
@@ -605,6 +788,31 @@  static int load_config(struct platform *p, struct config *config)
 	if (rc)
 		pb_log_fn("Failed to parse nvram\n");
 
+	/*
+	 * If we have an IPMI mailbox configuration available use it instead of
+	 * the boot order found in NVRAM.
+	 */
+	if (platform->get_ipmi_boot_mailbox) {
+		char *mailbox;
+		struct param *param;
+		rc = platform->get_ipmi_boot_mailbox(platform, &mailbox);
+		if (!rc && mailbox) {
+			platform->ipmi_mailbox_original_config =
+				talloc_strdup(
+					platform,
+					param_list_get_value(
+						platform->params, "petitboot,bootdevs"));
+			param_list_set(platform->params, "petitboot,bootdevs",
+					mailbox, false);
+			param = param_list_get_param(platform->params,
+					"petitboot,bootdevs");
+			/* Avoid writing this to NVRAM */
+			param->modified = false;
+			config->ipmi_bootdev_mailbox = true;
+			talloc_free(mailbox);
+		}
+	}
+
 	config_populate_all(config, platform->params);
 
 	if (platform->get_ipmi_bootdev) {
@@ -630,6 +838,7 @@  static int save_config(struct platform *p, struct config *config)
 {
 	struct platform_powerpc *platform = to_platform_powerpc(p);
 	struct config *defaults;
+	struct param *param;
 
 	if (config->ipmi_bootdev == IPMI_BOOTDEV_INVALID &&
 	    platform->clear_ipmi_bootdev) {
@@ -639,6 +848,23 @@  static int save_config(struct platform *p, struct config *config)
 		config->ipmi_bootdev_persistent = false;
 	}
 
+	if (!config->ipmi_bootdev_mailbox &&
+			platform->ipmi_mailbox_original_config) {
+		param = param_list_get_param(platform->params,
+				"petitboot,bootdevs");
+		/* Restore old boot order if unmodified */
+		if (!param->modified) {
+			param_list_set(platform->params, "petitboot,bootdevs",
+					platform->ipmi_mailbox_original_config,
+					false);
+			param->modified = false;
+			config_populate_bootdev(config, platform->params);
+		}
+		platform->clear_ipmi_boot_mailbox(platform);
+		talloc_free(platform->ipmi_mailbox_original_config);
+		platform->ipmi_mailbox_original_config = NULL;
+	}
+
 	defaults = talloc_zero(platform, struct config);
 	config_set_defaults(defaults);
 
@@ -718,6 +944,8 @@  static bool probe(struct platform *p, void *ctx)
 		platform->ipmi = ipmi_open(platform);
 		platform->get_ipmi_bootdev = get_ipmi_bootdev_ipmi;
 		platform->clear_ipmi_bootdev = clear_ipmi_bootdev_ipmi;
+		platform->get_ipmi_boot_mailbox = get_ipmi_boot_mailbox;
+		platform->clear_ipmi_boot_mailbox = clear_ipmi_boot_mailbox;
 		platform->set_os_boot_sensor = set_ipmi_os_boot_sensor;
 	} else if (!stat(sysparams_dir, &statbuf)) {
 		pb_debug("platform: using sysparams for IPMI paramters\n");
diff --git a/discover/platform.c b/discover/platform.c
index 237da3a9..8a23afd3 100644
--- a/discover/platform.c
+++ b/discover/platform.c
@@ -455,7 +455,7 @@  static int read_bootdev(void *ctx, char **pos, struct autoboot_option *opt)
 	return rc;
 }
 
-static void config_populate_bootdev(struct config *config,
+void config_populate_bootdev(struct config *config,
 	const struct param_list *pl)
 {
 	struct autoboot_option *opt, *new = NULL;
diff --git a/discover/platform.h b/discover/platform.h
index 29405626..1ac905af 100644
--- a/discover/platform.h
+++ b/discover/platform.h
@@ -27,6 +27,8 @@  const struct config *config_get(void);
 int config_set(struct config *config);
 void config_set_defaults(struct config *config);
 void config_set_autoboot(bool autoboot_enabled);
+void config_populate_bootdev(struct config *config,
+	const struct param_list *pl);
 void config_populate_all(struct config *config, const struct param_list *pl);
 
 void params_update_network_values(struct param_list *pl,