[Skiboot] [PATCH v3 04/10] opal-msg: Enhance opal-get-msg API

[Jeremy Kerr]

Hi Vasant,

> - If buffer size < actual message size, then opal_get_msg will copy
> partial data and return OPAL_PARTIAL to Linux.

This sounds a little dangerous, but I'm sure you had a good reason for
that (rather than dropping the message). Could you remind me of that?

> - Add new variable "extended" to "opal_msg_entry" structure to keep
> track of messages that has more than 64byte data. We will allocate
> separate memory for these messages and once kernel consumes message we
> will release that memory.

OK, that scheme sounds good.

> +	if (params_size > OPAL_MSG_FIXED_PARAMS_SIZE) {
> +		entry_size = sizeof(struct opal_msg_entry) + params_size;
> +		entry_size -= OPAL_MSG_FIXED_PARAMS_SIZE;
> +		entry = zalloc(entry_size);
>

Can we set entry->extended here ...

> +	} else {
> +		entry = list_pop(&msg_free_list, struct opal_msg_entry, link);
>  		if (!entry) {
> -			prerror("Allocation failed\n");
> -			unlock(&opal_msg_lock);
> -			return OPAL_RESOURCE;
> +			prerror("No available node in the free list, allocating\n");
> +			entry = zalloc(sizeof(struct opal_msg_entry));
>  		}
>
>  	}
> +	if (!entry) {
> +		prerror("Allocation failed\n");
> +		unlock(&opal_msg_lock);
> +		return OPAL_RESOURCE;
> +	}
>  
>  	entry->consumed = consumed;
>  	entry->data = data;
>  	entry->msg.msg_type = cpu_to_be32(msg_type);
> -
> -	if (params_size > OPAL_MSG_FIXED_PARAMS_SIZE) {
> -		prerror("Discarding extra parameters\n");
> -		params_size = OPAL_MSG_FIXED_PARAMS_SIZE;
> -	}
> +	entry->msg.size = cpu_to_be32(params_size);
>  	memcpy(entry->msg.params, params, params_size);
>  
> +	if (params_size > OPAL_MSG_FIXED_PARAMS_SIZE)
> +		entry->extended = true;

... rather than here, so that it's more obvious that ->extended is
correlated to the allocation we've just used. I don't want us to miss
that in a future change, as it'll result in a silent memory leak.

With that changed:

Acked-by: Jeremy Kerr <jk at ozlabs.org>

Cheers,


Jeremy