[Skiboot] [PATCH V2 19/21] core/pldm: Add file io read request
Abhishek SIngh Tomar
abhishek at linux.ibm.com
Wed Mar 16 20:23:20 AEDT 2022
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.
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