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

Christophe Lombard clombard at linux.vnet.ibm.com
Fri Mar 18 21:56:56 AEDT 2022


Le 16/03/2022 à 10:23, Abhishek SIngh Tomar a écrit :
> Hello Christophe
>
> In pldm_file_io_read_file()? How it size of data read successfully
> identified after function ends. My suggestion is to return Number of
> bytes read successfully currently we return PLDM_SUCCESS(0).
>
> Also if we expect that we read exactly same length of data we
> requested then we should add conditional check such as
> "size+offset> file_length"
> as this maybe one of case can lead to error in future.
That could be confusing, I think.
We read the exact number of bytes, as requested by the caller with the 
parameter "size".
The PLDM responder, running on the BMC, will return only the number
of bytes (size) you request.
> Regards
> Abhishek Singh Tomar
>
> On Fri, Mar 04, 2022 at 02:11:52PM +0100, 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 | 130 ++++++++++++++++++++++++++++++
>>   core/pldm/pldm.h                  |   4 +
>>   2 files changed, 134 insertions(+)
>>
>> diff --git a/core/pldm/pldm-file-io-requests.c b/core/pldm/pldm-file-io-requests.c
>> index f006c797..ce4297d0 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,135 @@ 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)
>> +
>> +/*
>> + * 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) && (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_do_request(BMC_EID, request_msg, sizeof(request_msg),
>> +				     &response_msg, &response_len);
>> +		if (rc) {
>> +			prlog(PR_ERR, "PLDM: Communication Error (req: ReadFileReq, rc: %d)\n", rc);
>> +			return OPAL_PARAMETER;
>> +		}
>> +
>> +		/* 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);
>> +			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)
>> +			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);
>> +	}
>> +
>> +	return OPAL_SUCCESS;
>> +}
>> +
>> +int pldm_file_io_read_file(uint32_t file_handle,
>> +			   uint32_t file_length,
>> +			   void *buf, uint32_t offset,
>> +			   uint64_t size)
>> +{
>> +	if (!file_io_ready)
>> +		return OPAL_PARAMETER;
>> +
>> +	return read_file_req(file_handle, file_length,
>> +			     buf, offset, size);
>> +}
>> +
>>   /*
>>    * Send/receive a PLDM GetFileTable request message.
>>    * The file table contains the list of files available and
>> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h
>> index 2695fa37..da572a6a 100644
>> --- a/core/pldm/pldm.h
>> +++ b/core/pldm/pldm.h
>> @@ -51,6 +51,10 @@ int pldm_rx_handle_request(struct pldm_rx_data *rx);
>>   int pldm_mctp_responder_init(void);
>>   
>>   /* Requester support */
>> +int pldm_file_io_read_file(uint32_t file_handle,
>> +			   uint32_t file_length,
>> +			   void *buf, uint32_t offset,
>> +			   uint64_t size);
>>   int pldm_file_io_init(void);
>>   
>>   int pldm_fru_get_record_by_option(uint16_t fru_table_handle,
>> -- 
>> 2.35.1
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot



More information about the Skiboot mailing list