[Skiboot] [PATCH RFC 08/12] opal-prd: Add support for variable-sized messages

[Vasant Hegde]

On 05/25/2017 12:35 PM, Jeremy Kerr wrote:
> With the introductuion of the opaque firmware channel, we want to
> support variable-sized messages. Rather than expecting to read an
> entire 'struct opal_prd_msg' in one read() call, we can split this
> over mutiple reads, potentially expanding our message buffer.
>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> ---
>  external/opal-prd/opal-prd.c | 67 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 55 insertions(+), 12 deletions(-)
>
> diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
> index c7cdc61..ebd6c0f 100644
> --- a/external/opal-prd/opal-prd.c
> +++ b/external/opal-prd/opal-prd.c
> @@ -80,6 +80,8 @@ struct opal_prd_ctx {
>  	char			*hbrt_file_name;
>  	bool			use_syslog;
>  	bool			expert_mode;
> +	struct opal_prd_msg	*msg;
> +	size_t			msg_alloc_len;
>  	void			(*vlog)(int, const char *, va_list);
>  };
>
> @@ -1225,41 +1227,76 @@ static int handle_msg_occ_reset(struct opal_prd_ctx *ctx,
>
>  static int handle_prd_msg(struct opal_prd_ctx *ctx)
>  {
> -	struct opal_prd_msg msg;
> +	struct opal_prd_msg *msg;
>  	int size;
>  	int rc;
>
> -	rc = read(ctx->fd, &msg, sizeof(msg));
> +	msg = ctx->msg;
> +
> +	rc = read(ctx->fd, msg, ctx->msg_alloc_len);
>  	if (rc < 0 && errno == EAGAIN)
>  		return -1;
>
> -	if (rc != sizeof(msg)) {
> -		pr_log(LOG_WARNING, "FW: Error reading events from OPAL: %m");
> +	/* we need at least enough for the message header... */
> +	if (rc < 0) {
> +		pr_log(LOG_WARNING, "FW: error reading from firmware: %m");
> +		return -1;
> +	}
> +
> +	if (rc < sizeof(msg->hdr)) {
> +		pr_log(LOG_WARNING, "FW: short message read from firmware");
>  		return -1;
>  	}
>
> -	size = htobe16(msg.hdr.size);
> -	if (size < sizeof(msg)) {
> +	/* ... and for the reported message size to be sane */
> +	size = htobe16(msg->hdr.size);
> +	if (size < sizeof(msg->hdr)) {
>  		pr_log(LOG_ERR, "FW: Mismatched message size "
>  				"between opal-prd and firmware "
>  				"(%d from FW, %zd expected)",
> -				size, sizeof(msg));
> +				size, sizeof(msg->hdr));
>  		return -1;
>  	}
>
> -	switch (msg.hdr.type) {
> +	/* expand our message buffer if necessary... */
> +	if (size > ctx->msg_alloc_len) {
> +		ctx->msg = msg = realloc(ctx->msg, size);

Validate memory allocation ?

> +		ctx->msg_alloc_len = size;
> +	}
> +
> +	/* ... and complete the read */
> +	if (size > rc) {
> +		size_t pos;
> +
> +		for (pos = rc; pos < size;) {
> +			rc = read(ctx->fd, msg + pos, size - pos);
> +
> +			if (rc < 0 && errno == EAGAIN)
> +				continue;
> +
> +			if (rc <= 0) {
> +				pr_log(LOG_WARNING,
> +					"FW: error reading from firmware: %m");
> +				return -1;
> +			}
> +
> +			pos += rc;
> +		}
> +	}
> +
> +	switch (msg->hdr.type) {
>  	case OPAL_PRD_MSG_TYPE_ATTN:
> -		rc = handle_msg_attn(ctx, &msg);
> +		rc = handle_msg_attn(ctx, msg);
>  		break;
>  	case OPAL_PRD_MSG_TYPE_OCC_RESET:
> -		rc = handle_msg_occ_reset(ctx, &msg);
> +		rc = handle_msg_occ_reset(ctx, msg);
>  		break;
>  	case OPAL_PRD_MSG_TYPE_OCC_ERROR:
> -		rc = handle_msg_occ_error(ctx, &msg);
> +		rc = handle_msg_occ_error(ctx, msg);
>  		break;
>  	default:
>  		pr_log(LOG_WARNING, "Invalid incoming message type 0x%x",
> -				msg.hdr.type);
> +				msg->hdr.type);
>  		return -1;
>  	}
>
> @@ -1621,6 +1658,10 @@ static int run_prd_daemon(struct opal_prd_ctx *ctx)
>  	ctx->fd = -1;
>  	ctx->socket = -1;
>
> +	/* set up our message buffer */
> +	ctx->msg_alloc_len = sizeof(*ctx->msg);
> +	ctx->msg = malloc(ctx->msg_alloc_len);

Validate malloc return value ?

-Vasant