From 01acbed66c661ce1125ebe6342b7674d3e11cdcd Mon Sep 17 00:00:00 2001 From: Raghavendra Rao Ananta Date: Tue, 24 Mar 2020 06:48:51 -0700 Subject: [PATCH] haven: hh_msgq: Let clients manage the buffers for hh_msgq_send Currently, the clients of the hh_msgq driver would allocate buffers, fill with data and call hh_msgq_send(). Upon success, hh_msgq_send() would be freeing the buffer, while in the case of failure, the clients are responsible for it. However, this approach of buffer management is not symmetric and could be confusing for the clients. Hence, let the clients take full ownership of the buffers- allocating and freeing. Also make changes to the affected client drivers. Change-Id: I73101d823f9d3de16414e444fde494222f584e53 Signed-off-by: Raghavendra Rao Ananta --- drivers/soc/qcom/mem-buf.c | 30 +++++++++++++++++++++++------- drivers/virt/haven/hh_msgq.c | 13 ++----------- drivers/virt/haven/hh_rm_core.c | 13 ++++++++++--- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/drivers/soc/qcom/mem-buf.c b/drivers/soc/qcom/mem-buf.c index 78e0cfaf21d2..d3c3689ad684 100644 --- a/drivers/soc/qcom/mem-buf.c +++ b/drivers/soc/qcom/mem-buf.c @@ -662,10 +662,16 @@ static void mem_buf_alloc_req_work(struct work_struct *work) resp_msg->ret = ret; ret = hh_msgq_send(mem_buf_hh_msgq_hdl, resp_msg, sizeof(*resp_msg), 0); + + /* + * Free the buffer regardless of the return value as the hypervisor + * would have consumed the data in the case of a success. + */ + kfree(resp_msg); + if (ret < 0) { pr_err("%s: failed to send memory allocation response rc: %d\n", __func__, ret); - kfree(resp_msg); mutex_lock(&mem_buf_xfer_mem_list_lock); list_del(&xfer_mem->entry); mutex_unlock(&mem_buf_xfer_mem_list_lock); @@ -868,10 +874,15 @@ static int mem_buf_request_mem(struct mem_buf_desc *membuf) } ret = mem_buf_msg_send(alloc_req_msg, msg_size); - if (ret < 0) { - kfree(alloc_req_msg); + + /* + * Free the buffer regardless of the return value as the hypervisor + * would have consumed the data in the case of a success. + */ + kfree(alloc_req_msg); + + if (ret < 0) goto out; - } ret = mem_buf_txn_wait(&txn); if (ret < 0) @@ -897,11 +908,16 @@ static void mem_buf_relinquish_mem(struct mem_buf_desc *membuf) msg->hdl = membuf->memparcel_hdl; ret = hh_msgq_send(mem_buf_hh_msgq_hdl, msg, sizeof(*msg), 0); - if (ret < 0) { + + /* + * Free the buffer regardless of the return value as the hypervisor + * would have consumed the data in the case of a success. + */ + kfree(msg); + + if (ret < 0) pr_err("%s failed to send memory relinquish message rc: %d\n", __func__, ret); - kfree(msg); - } } static int mem_buf_map_mem_s2(struct mem_buf_desc *membuf) diff --git a/drivers/virt/haven/hh_msgq.c b/drivers/virt/haven/hh_msgq.c index c2aa58ab843e..575f8211bc81 100644 --- a/drivers/virt/haven/hh_msgq.c +++ b/drivers/virt/haven/hh_msgq.c @@ -239,15 +239,12 @@ static int __hh_msgq_send(struct hh_msgq_cap_table *cap_table_entry, /** * hh_msgq_send: Send a message to the client on a different VM * @client_desc: The client descriptor that was obtained via hh_msgq_register() - * @buff: Pointer to the buffer that needs to be sent. The buffer should be - * dynamically allocated via kmalloc/kzalloc. + * @buff: Pointer to the buffer that needs to be sent * @size: The size of the buffer * @flags: Optional flags to pass to send the data. For the list of flags, * see linux/haven/hh_msgq.h * - * The function would free the buffer upon success, and returns 0. The caller - * should not be referencing the buffer anymore. - * On the other hand, it returns -EINVAL if the caller passes invalid arguments, + * The function returns -EINVAL if the caller passes invalid arguments, * -EAGAIN if the message queue is not yet ready to communicate, and -EPERM if * the caller doesn't have permissions to send the data. * @@ -312,12 +309,6 @@ int hh_msgq_send(void *msgq_client_desc, ret = __hh_msgq_send(cap_table_entry, buff, size, flags); } while (ret == -EAGAIN); - /* If the send is success, hypervisor should not be holding any - * references to 'buff', and hence, can be released. - */ - if (!ret) - kfree(buff); - return ret; err: spin_unlock(&cap_table_entry->cap_entry_lock); diff --git a/drivers/virt/haven/hh_rm_core.c b/drivers/virt/haven/hh_rm_core.c index 25d0ff88dde5..d0c7db026dff 100644 --- a/drivers/virt/haven/hh_rm_core.c +++ b/drivers/virt/haven/hh_rm_core.c @@ -518,10 +518,17 @@ static int hh_rm_send_request(u32 message_id, ret = hh_msgq_send(hh_rm_msgq_desc, send_buff, sizeof(*hdr) + payload_size, tx_flags); - if (ret) { - kfree(send_buff); + + /* + * In the case of a success, the hypervisor would have consumed + * the buffer. While in the case of a failure, we are going to + * quit anyways. Hence, free the buffer regardless of the + * return value. + */ + kfree(send_buff); + + if (ret) return ret; - } } return 0;