[Skiboot] [PATCH V6 19/21] core/pldm: Add file io read request

[Frederic Barrat]

On 13/09/2022 12:27, Christophe Lombard wrote:
> Send/receive a PLDM ReadFile request message.
> 
> Due to maximum transfer size for PLDM protocol, we have to send several
> read requests, if necessary.
> 
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
> ---
>   core/pldm/pldm-file-io-requests.c | 129 ++++++++++++++++++++++++++++++
>   core/pldm/pldm.h                  |   2 +
>   2 files changed, 131 insertions(+)
> 
> diff --git a/core/pldm/pldm-file-io-requests.c b/core/pldm/pldm-file-io-requests.c
> index 5eb260d6..48b00ef2 100644
> --- a/core/pldm/pldm-file-io-requests.c
> +++ b/core/pldm/pldm-file-io-requests.c
> @@ -7,6 +7,7 @@
>   #include <stdio.h>
>   #include <string.h>
>   #include <inttypes.h>
> +#include <timebase.h>
>   #include <pldm/ibm/libpldm/file_io.h>
>   #include "pldm.h"
>   
> @@ -33,6 +34,134 @@ static void file_io_init_complete(bool success)
>   	file_io_ready = true;
>   }
>   
> +/* maximum currently transfer size for PLDM */
> +#define KILOBYTE 1024ul
> +#define MAX_TRANSFER_SIZE_BYTES (127 * KILOBYTE + 1)


I didn't find the origin of that limit, especially the odd-looking "+1"...


> +
> +/*
> + * Send/receive a PLDM ReadFile request message.
> + */
> +static int read_file_req(uint32_t file_handle, uint32_t file_length,
> +			 void *buf, uint32_t offset, uint64_t size)
> +{
> +	char request_msg[PKT_SIZE(struct pldm_read_file_req)];
> +	size_t response_len, payload_len, file_data_offset;
> +	uint8_t completion_code;
> +	uint32_t resp_length;
> +	uint64_t total_read;
> +	void *response_msg;
> +	int num_transfers;
> +	void *curr_buf;
> +	int rc, i;
> +
> +	struct pldm_read_file_req file_req = {
> +		.file_handle = file_handle,
> +		.offset = offset,
> +		.length = size
> +	};
> +
> +	if (!file_length)
> +		return OPAL_PARAMETER;
> +
> +	if ((!size) || (size > file_length))
> +		return OPAL_PARAMETER;
> +
> +	if ((offset) && ((size + offset) > file_length))
> +		return OPAL_PARAMETER;


The last 2 'if' statements should be equivalent to:
	if ((!size) || ((size + offset) > file_length))
		return OPAL_PARAMETER;


> +
> +	num_transfers = 1;
> +	curr_buf = buf;
> +	total_read = 0;
> +
> +	if (file_req.length > MAX_TRANSFER_SIZE_BYTES) {
> +		num_transfers = (file_req.length + MAX_TRANSFER_SIZE_BYTES - 1) /
> +				MAX_TRANSFER_SIZE_BYTES;
> +		file_req.length = MAX_TRANSFER_SIZE_BYTES;
> +	}
> +
> +	prlog(PR_TRACE, "%s - file_handle: %d, offset: 0x%x, size: 0x%llx num_transfers: %d\n",
> +			__func__, file_handle, file_req.offset,
> +			size, num_transfers);
> +
> +	for (i = 0; i < num_transfers; i++) {
> +		file_req.offset = offset + (i * MAX_TRANSFER_SIZE_BYTES);
> +
> +		/* Encode the file request */
> +		rc = encode_read_file_req(
> +				DEFAULT_INSTANCE_ID,
> +				file_req.file_handle,
> +				file_req.offset,
> +				file_req.length,
> +				(struct pldm_msg *)request_msg);
> +		if (rc != PLDM_SUCCESS) {
> +			prlog(PR_ERR, "Encode ReadFileReq Error, rc: %d\n",
> +				      rc);
> +			return OPAL_PARAMETER;
> +		}
> +
> +		/* Send and get the response message bytes */
> +		rc = pldm_requester_queue_and_wait(
> +					request_msg, sizeof(request_msg),
> +					&response_msg, &response_len);
> +		if (rc) {
> +			prlog(PR_ERR, "Communication Error, req: ReadFileReq, rc: %d\n", rc);
> +			return rc;
> +		}
> +
> +		/* Decode the message */
> +		payload_len = response_len - sizeof(struct pldm_msg_hdr);
> +		rc = decode_read_file_resp(
> +				response_msg,
> +				payload_len,
> +				&completion_code,
> +				&resp_length,
> +				&file_data_offset);
> +		if (rc != PLDM_SUCCESS || completion_code != PLDM_SUCCESS) {
> +			prlog(PR_ERR, "Decode ReadFileResp Error, rc: %d, cc: %d\n",
> +				      rc, completion_code);
> +			free(response_msg);
> +			return OPAL_PARAMETER;
> +		}
> +
> +		if (resp_length == 0) {
> +			free(response_msg);
> +			break;
> +		}
> +
> +		memcpy(curr_buf,
> +		       ((struct pldm_msg *)response_msg)->payload + file_data_offset,
> +		       resp_length);
> +
> +		total_read += resp_length;
> +		curr_buf += resp_length;
> +		free(response_msg);
> +
> +		prlog(PR_TRACE, "%s - file_handle: %d, resp_length: 0x%x, total_read: 0x%llx\n",
> +			__func__, file_handle, resp_length, total_read);
> +
> +		if (total_read == size)


if (total_read >= size) for peace of mind?


> +			break;
> +		else if (resp_length != file_req.length) {
> +			/* end of file */
> +			break;
> +		} else if (MAX_TRANSFER_SIZE_BYTES > (size - total_read))
> +			file_req.length = size - total_read;
> +
> +		time_wait_ms(20);


Any reason for that 20 ms delay?

   Fred