[Skiboot] [PATCH v2 4/5] external/opal-prd: Support manufacturing command HTMGT and attribute override

[Jeremy Kerr]

Hi Neelesh,


> diff --git a/external/opal-prd/hostboot-interface.h b/external/opal-prd/hostboot-interface.h
> index a518f50..7ab928f 100644
> --- a/external/opal-prd/hostboot-interface.h
> +++ b/external/opal-prd/hostboot-interface.h
> @@ -421,6 +421,41 @@ struct runtime_interfaces {
>  	 */
>  	int (*enable_occ_actuation)(bool i_occActivation);
>  
> +	/**
> +	 * @brief	Apply a set of attribute overrides
> +	 *
> +	 * @param[in]	pointer to binary override data
> +	 * @param[in]	length of override data (bytes)
> +	 * @returns	0 on success, or return code if the command failed
> +	 *
> +	 * @platform	OpenPower
> +	 */
> +	int (*apply_attr_override)(uint8_t *i_data, size_t size);
> +
> +	/**
> +	 * @brief	Send a pass-through command to HTMGT
> +	 *
> +	 * @details	This is a blocking call that will send a command to
> +	 *		HTMGT.
> +	 *
> +	 * @note	If o_rspLength is returned with a non-zero value, the
> +	 *		data at the o_rspData should be dumped to stdout in a
> +	 *		hex dump format.
> +	 * @note	The maximum response data returned will be 4096 bytes
> +	 *
> +	 * @param[in]	i_cmdLength	number of bytes in pass-thru command data
> +	 * @param[in]	*i_cmdData	pointer to pass-thru command data
> +	 * @param[out]	*o_rspLength	pointer to number of bytes returned in
> +	 *				o_rspData
> +	 * @param[out]	*o_rspData	pointer to a 4096 byte buffer that will
> +	 *				contain the response data from the command
> +	 *
> +	 * @returns	0 on success, or return code if the command failed
> +	 * @platform	OpenPower
> +	 */
> +	int (*htmgt_pass_thru)(uint16_t i_cmdLength, uint8_t *i_cmdData,
> +			       uint16_t *o_rspLength, uint8_t *o_rspData);
> +
>  	/* Reserve some space for future growth. */
>  	void (*reserved[32])(void);

Since we don't have interface version changes here, you'll need to
consume part of this reserved[] array if you add functions to the
runtime_interfaces struct.

> diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
> index 8f1d6cb..9b9e213 100644
> --- a/external/opal-prd/opal-prd.c
> +++ b/external/opal-prd/opal-prd.c
> @@ -75,6 +75,7 @@ struct opal_prd_ctx {
>  	struct pnor		pnor;
>  	char			*hbrt_file_name;
>  	bool			use_syslog;
> +	bool			mfg_mode;
>  	void			(*vlog)(int, const char *, va_list);
>  };
>  
> @@ -83,13 +84,20 @@ enum control_msg_type {
>  	CONTROL_MSG_DISABLE_OCCS	= 0x01,
>  	CONTROL_MSG_TEMP_OCC_RESET	= 0x02,
>  	CONTROL_MSG_TEMP_OCC_ERROR	= 0x03,
> +	CONTROL_MSG_ATTR_OVERRIDE	= 0x10,
> +	CONTROL_MSG_HTMGT_PASSTHRU	= 0x20,
>  };

Why the gap in numbering here?

>  
>  struct control_msg {
>  	enum control_msg_type	type;
> -	uint64_t		response;
> +	int			response;
> +	uint32_t		argc;
> +	uint32_t		data_len;
> +	uint8_t			data[];
>  };
>  
> +#define MAX_CONTROL_MSG_BUF	4096
> +
>  static struct opal_prd_ctx *ctx;
>  
>  static const char *opal_prd_devnode = "/dev/opal-prd";
> @@ -179,6 +187,22 @@ static void pr_log_nocall(const char *name)
>  	pr_log(LOG_WARNING, "HBRT: Call %s not provided", name);
>  }
>  
> +static void pr_log_stream(const uint8_t *data, uint32_t len)

Not sure this function name matches what the function does. How about
just hexdump() ?



> +	printf("\t%s htmgt-passthru <1st-byte> <2nd byte>..<nth byte>\n",
> +			progname);


Just:
        printf("\t%s htmgt-passthru <bytes...>\n", progname);


> +	printf("\t%s override <binary-blob>\n", progname);

and:

        printf("\t%s override <FILE>\n", progname);

(binary-blob doesn't tell the user that this is supposed to refer to a file)

> +	{"mfg-mode", no_argument, NULL, 'm'},

You listed --manufacting-mode in the commit header here. I prefer that,
but either is fine.


> @@ -1483,8 +1723,29 @@ int main(int argc, char *argv[])
>  		}
>  
>  		rc = send_occ_control(ctx, argv[optind + 1]);
> +		break;
> +	case ACTION_ATTR_OVERRIDE:
> +		if (optind + 1 >= argc) {
> +			pr_log(LOG_ERR, "CTRL: attribute override command "
> +					"requires an argument");
> +			return EXIT_FAILURE;
> +		}
> +
> +		rc = send_attr_override(ctx, argc - optind - 1, &argv[optind + 1]);
> +		break;
> +	case ACTION_HTMGT_PASSTHRU:
> +		if (optind + 1 >= argc) {
> +			pr_log(LOG_ERR, "CTRL: htmgt passthru requires minimum "
> +					"an argument");

English nitpick: s/minimum an/at least one/

Cheers,


Jeremy