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

[Christophe Lombard]

Le 12/04/2023 à 17:49, Frederic Barrat a écrit :
>
>
> 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"...
>

I had read somewhere that the maximum transfer size for PLDM is defined 
as 180K and
HB has found that the optimal transfer size that doesn't make HB crash 
is (127k + 1) bytes.

>
>> +
>> +/*
>> + * 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;
>

good. Thanks

>
>> +
>> +    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?
>

right, :-)

>
>> +            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?
>

Really, I don't remember. May be, we could remove this delay.

> Fred
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20230418/9863f09f/attachment.htm>