[Skiboot] [PATCH V6 20/21] core/pldm: Add file io write request
Abhishek SIngh Tomar
abhishek at linux.ibm.com
Fri Sep 30 16:27:49 AEST 2022
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?
> +
> + 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
> +
> + 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
> + 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