[Skiboot] [PATCH v5 2/3] platforms/astbmc: Check for SBE validation step

[Vasant Hegde]

On 05/07/2019 07:31 AM, Samuel Mendoza-Jonas wrote:
> On some POWER8 astbmc systems an update to the SBE requires pausing at
> runtime to ensure integrity of the SBE. If this is required the BMC will
> set a chassis boot option IPMI flag using the OEM parameter 0x62. If
> Skiboot sees this flag is set it waits until the SBE update is complete
> and the flag is cleared.
> Unfortunately the mystery operation that validates the SBE also leaves
> it in a bad state and unable to be used for timer operations. To
> workaround this the flag is checked as soon as possible (ie. when IPMI
> and the console are set up), and once complete the system is rebooted.

Keep in mind that the way we do timer init is different in P8 and P9.
In P9, p9_sbe_init() initiates timer while in P8, slw_init() initiates timer.

Your platform.seeprom_update() works fine on P8 systems today. We have to be 
careful
before enabling on P9 systems.


> 
> Signed-off-by: Samuel Mendoza-Jonas <sam at mendozajonas.com>
> ---


.../...

> @@ -110,3 +134,78 @@ int ipmi_get_bmc_info_request(void)
>   	bmc_info_waiting = true;
>   	return rc;
>   }
> +
> +/* This will free ipmi_sys_boot_opt structure */
> +int ipmi_chassis_check_sbe_validation(void)
> +{
> +	int rc = -1;
> +
> +	while (bmc_boot_opt_waiting)
> +		time_wait_ms(5);

With 5ms we may not run poller (see time_wait_poll()).

> +
> +	if (!bmc_boot_opt_valid)

Free ipmi_sys_boot_opt and return rc?

> +		return -1;
> +
> +	if ((ipmi_sys_boot_opt->param_valid & 0x8) != 0)
> +		goto out;
> +	if (ipmi_sys_boot_opt->param_valid != 0x62)
> +		goto out;
> +
> +	rc = ipmi_sys_boot_opt->flag_set;
> +
> +out:
> +	free(ipmi_sys_boot_opt);
> +	return rc;
> +}
> +
> +static void ipmi_get_chassis_boot_opt_resp(struct ipmi_msg *msg)
> +{
> +	bmc_boot_opt_waiting = false;
> +
> +	if (msg->cc != IPMI_CC_NO_ERROR) {
> +		prlog(PR_INFO, "IPMI: IPMI_CHASSIS_GET_BOOT_OPT cmd returned error"
> +		      " [rc : 0x%x]\n", msg->data[0]);

Free ipmi message here ?

> +		return;
> +	}
> +
> +	if (msg->resp_size == sizeof(struct ipmi_sys_boot_opt)) {
> +		bmc_boot_opt_valid = true;
> +		memcpy(ipmi_sys_boot_opt, msg->data, msg->resp_size);
> +	} else {
> +		prlog(PR_WARNING, "IPMI: IPMI_CHASSIS_GET_BOOT_OPT unexpected response size\n");
> +	}
> +
> +	ipmi_free_msg(msg);
> +}
> +


.../...

>   extern void slot_table_init(const struct slot_table_entry *top_table);
>   extern void slot_table_get_slot_info(struct phb *phb, struct pci_device * pd);
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index faa73e2f..413613da 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -26,6 +26,7 @@
>   #include <bt.h>
>   #include <errorlog.h>
>   #include <lpc.h>
> +#include <timebase.h>
> 
>   #include "astbmc.h"
> 
> @@ -168,6 +169,52 @@ int64_t astbmc_ipmi_reboot(void)
>   	return ipmi_chassis_control(IPMI_CHASSIS_HARD_RESET);
>   }
> 
> +void astbmc_seeprom_update(void)
> +{
> +	int flag_set, counter;
> +
> +	ipmi_get_chassis_boot_opt_request();
> +
> +	flag_set = ipmi_chassis_check_sbe_validation();
> +
> +	if (flag_set <= 0) {
> +		prlog(PR_DEBUG, "SBE validation flag unset or invalid\n");
> +		return;
> +	}
> +
> +	/*
> +	 * Flag is set, wait until SBE validation is complete and the flag
> +	 * has been reset.
> +	 */
> +	prlog(PR_WARNING, "SBE validation required, waiting for completion\n");
> +	prlog(PR_WARNING, "System will be powered off if validation fails\n");
> +	counter = 0;
> +
> +	while (flag_set > 0) {

What is BMC never updates option flag properly (say buggy BMC code) ?
Do we need some way to exit this loop ?

> +		time_wait_ms(10000);
> +		if (++counter >= 3) {
> +			/* Let the user know we're alive every 30s */
> +			prlog(PR_WARNING, "waiting for completion..\n");
> +			counter = 0;
> +		}
> +
> +		ipmi_get_chassis_boot_opt_request();

May be its good to check return value to confirm we did queue IPMI message properly.

-Vasant

> +		flag_set = ipmi_chassis_check_sbe_validation();
> +	}
> +