[Skiboot] [PATCH V6 20/21] core/pldm: Add file io write request
Christophe Lombard
clombard at linux.vnet.ibm.com
Thu Oct 27 20:25:44 AEDT 2022
Le 30/09/2022 à 08:27, Abhishek SIngh Tomar a écrit :
> Hello Christophe
> I am having some questions in this patch
>> +
>> + if ((offset) && (offset > size))
>> + return OPAL_PARAMETER;
> Why is offset need to be lesser than size of buffer to write?
Good catch ! The test is not correct. The good solution is:
if ((offset) && ((size + offset) > file_length))
return OPAL_PARAMETER;
>> +
>> + request_length = sizeof(struct pldm_msg_hdr) +
>> + sizeof(struct pldm_write_file_req) +
>> + size;
> Why request payload is equal to size (size of buffer)?
> The Maximum request payload needed when size > MAX_TRANSFER_SIZE_BYTES
> is MAX_TRANSFER_SIZE_BYTES
>
request_length is the size of the complete PLDM request you send to the BMC.
We need to add the size of the common header + the specific header + the
size of lid file requested.
>> +
>> + for (i = 0; i < num_transfers; i++) {
>> + file_req.offset = offset + (i * MAX_TRANSFER_SIZE_BYTES);
>> +
>> + /* Encode the file request */
>> + rc = encode_write_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 WriteFileReq Error, rc: %d\n", rc);
>> + free(request_msg);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + /* Send and get the response message bytes */
>> + rc = pldm_requester_queue_and_wait(
>> + request_msg, request_length - 1,
>> + &response_msg, &response_len);
>> + if (rc) {
>> + prlog(PR_ERR, "Communication Error, req: WriteFileReq, rc: %d\n", rc);
>> + free(request_msg);
>> + return rc;
>> + }
>> +
>> + /* Decode the message */
>> + payload_len = response_len - sizeof(struct pldm_msg_hdr);
>> + rc = decode_write_file_resp(
>> + response_msg,
>> + payload_len,
>> + &completion_code,
>> + &resp_length);
>> + if (rc != PLDM_SUCCESS || completion_code != PLDM_SUCCESS) {
>> + prlog(PR_ERR, "Decode WriteFileResp Error, rc: %d, cc: %d\n",
>> + rc, completion_code);
>> + free(request_msg);
>> + free(response_msg);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + if (resp_length == 0) {
>> + free(response_msg);
>> + break;
>> + }
>> +
>> + total_write += resp_length;
>> + current_ptr += resp_length;
>> + free(response_msg);
>> +
> In this loop, if size > MAX_TRANSFER_SIZE_BYTES, after sending
> MAX_TRANSFER_SIZE_BYTES once how is payload pointer increasing?
> I think we should copy current_ptr to request_msg payload
You are right. Good point.
>> + if (total_write == size)
>> + break;
>> + else if (resp_length != file_req.length) {
>> + /* end of file */
>> + break;
>> + } else if (MAX_TRANSFER_SIZE_BYTES > (size - total_write))
>> + file_req.length = size - total_write;
>> + }
>> +
> Reviewed-by: Abhishek Singh Tomar <abhishek at linux.ibm.com>
More information about the Skiboot
mailing list