From 19c5b89d8d582cce9a90335de212cf477fe15b95 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Jul 2017 16:08:29 -0400 Subject: vmci: fix buf_size in case of iovec-based accesses Both qp_dequeue_locked() and qp_enqueue_locked() use the buf_size argument to decide how much would be there to copy; in case of iovec- (== msghdr-)based primitives it's not iov_size, it's msg_data_left(msg). Signed-off-by: Al Viro --- drivers/misc/vmw_vmci/vmci_queue_pair.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c index 8af5c2672f71..7387dedcda67 100644 --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c @@ -3253,7 +3253,7 @@ ssize_t vmci_qpair_enquev(struct vmci_qp *qpair, result = qp_enqueue_locked(qpair->produce_q, qpair->consume_q, qpair->produce_q_size, - msg, iov_size, + msg, msg_data_left(msg), qp_memcpy_to_queue_iov); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && @@ -3295,7 +3295,7 @@ ssize_t vmci_qpair_dequev(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - msg, iov_size, + msg, msg_data_left(msg), qp_memcpy_from_queue_iov, true); @@ -3339,7 +3339,7 @@ ssize_t vmci_qpair_peekv(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - msg, iov_size, + msg, msg_data_left(msg), qp_memcpy_from_queue_iov, false); -- cgit From ce3d6e7d4200617e6086c1a83220cf6efd3c92cf Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Jul 2017 16:13:07 -0400 Subject: vmci: get rid of qp_memcpy_from_queue() switch both of its users to qp_memcpy_from_queue_iov() - just make it take iov_iter * instead of msghdr * and arrange for a iov_iter for it in all cases. Signed-off-by: Al Viro --- drivers/misc/vmw_vmci/vmci_queue_pair.c | 37 +++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c index 7387dedcda67..a0c10f8cba30 100644 --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c @@ -441,13 +441,11 @@ static int __qp_memcpy_from_queue(void *dest, to_copy = size - bytes_copied; if (is_iovec) { - struct msghdr *msg = dest; + struct iov_iter *to = dest; int err; - /* The iovec will track bytes_copied internally. */ - err = memcpy_to_msg(msg, (u8 *)va + page_offset, - to_copy); - if (err != 0) { + err = copy_to_iter((u8 *)va + page_offset, to_copy, to); + if (err != to_copy) { if (kernel_if->host) kunmap(kernel_if->u.h.page[page_index]); return VMCI_ERROR_INVALID_ARGS; @@ -577,15 +575,6 @@ static int qp_memcpy_to_queue(struct vmci_queue *queue, (u8 *)src + src_offset, size, false); } -static int qp_memcpy_from_queue(void *dest, - size_t dest_offset, - const struct vmci_queue *queue, - u64 queue_offset, size_t size) -{ - return __qp_memcpy_from_queue((u8 *)dest + dest_offset, - queue, queue_offset, size, false); -} - /* * Copies from a given iovec from a VMCI Queue. */ @@ -3159,18 +3148,22 @@ ssize_t vmci_qpair_dequeue(struct vmci_qp *qpair, int buf_type) { ssize_t result; + struct iov_iter to; + struct kvec v = {.iov_base = buf, .iov_len = buf_size}; if (!qpair || !buf) return VMCI_ERROR_INVALID_ARGS; + iov_iter_kvec(&to, READ | ITER_KVEC, &v, 1, buf_size); + qp_lock(qpair); do { result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - buf, buf_size, - qp_memcpy_from_queue, true); + &to, buf_size, + qp_memcpy_from_queue_iov, true); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3200,19 +3193,23 @@ ssize_t vmci_qpair_peek(struct vmci_qp *qpair, size_t buf_size, int buf_type) { + struct iov_iter to; + struct kvec v = {.iov_base = buf, .iov_len = buf_size}; ssize_t result; if (!qpair || !buf) return VMCI_ERROR_INVALID_ARGS; + iov_iter_kvec(&to, READ | ITER_KVEC, &v, 1, buf_size); + qp_lock(qpair); do { result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - buf, buf_size, - qp_memcpy_from_queue, false); + &to, buf_size, + qp_memcpy_from_queue_iov, false); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3295,7 +3292,7 @@ ssize_t vmci_qpair_dequev(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - msg, msg_data_left(msg), + &msg->msg_iter, msg_data_left(msg), qp_memcpy_from_queue_iov, true); @@ -3339,7 +3336,7 @@ ssize_t vmci_qpair_peekv(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - msg, msg_data_left(msg), + &msg->msg_iter, msg_data_left(msg), qp_memcpy_from_queue_iov, false); -- cgit From 53f58d8ed862a3675659fb3162abfc5dd5f1025b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Jul 2017 16:42:29 -0400 Subject: vmci: simplify qp_dequeue_locked() * no need for callback argument - it's always the same one * fold __qp_memcpy_from_queue() into its only caller, get rid of dead code * pass struct iov_iter * without casting to void * * don't pass buf_size at all - it's always iov_iter_count(to) Signed-off-by: Al Viro --- drivers/misc/vmw_vmci/vmci_queue_pair.c | 71 +++++++++------------------------ 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c index a0c10f8cba30..09314b991fdf 100644 --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c @@ -142,9 +142,6 @@ typedef int vmci_memcpy_to_queue_func(struct vmci_queue *queue, u64 queue_offset, const void *src, size_t src_offset, size_t size); -typedef int vmci_memcpy_from_queue_func(void *dest, size_t dest_offset, - const struct vmci_queue *queue, - u64 queue_offset, size_t size); /* The Kernel specific component of the struct vmci_queue structure. */ struct vmci_queue_kern_if { @@ -411,11 +408,9 @@ static int __qp_memcpy_to_queue(struct vmci_queue *queue, * by traversing the offset -> page translation structure for the queue. * Assumes that offset + size does not wrap around in the queue. */ -static int __qp_memcpy_from_queue(void *dest, - const struct vmci_queue *queue, - u64 queue_offset, - size_t size, - bool is_iovec) +static int qp_memcpy_from_queue_iter(struct iov_iter *to, + const struct vmci_queue *queue, + u64 queue_offset, size_t size) { struct vmci_queue_kern_if *kernel_if = queue->kernel_if; size_t bytes_copied = 0; @@ -427,6 +422,7 @@ static int __qp_memcpy_from_queue(void *dest, (queue_offset + bytes_copied) & (PAGE_SIZE - 1); void *va; size_t to_copy; + int err; if (kernel_if->host) va = kmap(kernel_if->u.h.page[page_index]); @@ -440,21 +436,12 @@ static int __qp_memcpy_from_queue(void *dest, else to_copy = size - bytes_copied; - if (is_iovec) { - struct iov_iter *to = dest; - int err; - - err = copy_to_iter((u8 *)va + page_offset, to_copy, to); - if (err != to_copy) { - if (kernel_if->host) - kunmap(kernel_if->u.h.page[page_index]); - return VMCI_ERROR_INVALID_ARGS; - } - } else { - memcpy((u8 *)dest + bytes_copied, - (u8 *)va + page_offset, to_copy); + err = copy_to_iter((u8 *)va + page_offset, to_copy, to); + if (err != to_copy) { + if (kernel_if->host) + kunmap(kernel_if->u.h.page[page_index]); + return VMCI_ERROR_INVALID_ARGS; } - bytes_copied += to_copy; if (kernel_if->host) kunmap(kernel_if->u.h.page[page_index]); @@ -591,21 +578,6 @@ static int qp_memcpy_to_queue_iov(struct vmci_queue *queue, return __qp_memcpy_to_queue(queue, queue_offset, msg, size, true); } -/* - * Copies to a given iovec from a VMCI Queue. - */ -static int qp_memcpy_from_queue_iov(void *dest, - size_t dest_offset, - const struct vmci_queue *queue, - u64 queue_offset, size_t size) -{ - /* - * We ignore dest_offset because dest is really a struct iovec * and - * will maintain offset internally. - */ - return __qp_memcpy_from_queue(dest, queue, queue_offset, size, true); -} - /* * Allocates kernel VA space of specified size plus space for the queue * and kernel interface. This is different from the guest queue allocator, @@ -2679,11 +2651,10 @@ static ssize_t qp_enqueue_locked(struct vmci_queue *produce_q, static ssize_t qp_dequeue_locked(struct vmci_queue *produce_q, struct vmci_queue *consume_q, const u64 consume_q_size, - void *buf, - size_t buf_size, - vmci_memcpy_from_queue_func memcpy_from_queue, + struct iov_iter *to, bool update_consumer) { + size_t buf_size = iov_iter_count(to); s64 buf_ready; u64 head; size_t read; @@ -2705,15 +2676,15 @@ static ssize_t qp_dequeue_locked(struct vmci_queue *produce_q, read = (size_t) (buf_ready > buf_size ? buf_size : buf_ready); head = vmci_q_header_consumer_head(produce_q->q_header); if (likely(head + read < consume_q_size)) { - result = memcpy_from_queue(buf, 0, consume_q, head, read); + result = qp_memcpy_from_queue_iter(to, consume_q, head, read); } else { /* Head pointer wraps around. */ const size_t tmp = (size_t) (consume_q_size - head); - result = memcpy_from_queue(buf, 0, consume_q, head, tmp); + result = qp_memcpy_from_queue_iter(to, consume_q, head, tmp); if (result >= VMCI_SUCCESS) - result = memcpy_from_queue(buf, tmp, consume_q, 0, + result = qp_memcpy_from_queue_iter(to, consume_q, 0, read - tmp); } @@ -3162,8 +3133,7 @@ ssize_t vmci_qpair_dequeue(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - &to, buf_size, - qp_memcpy_from_queue_iov, true); + &to, true); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3208,8 +3178,7 @@ ssize_t vmci_qpair_peek(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - &to, buf_size, - qp_memcpy_from_queue_iov, false); + &to, false); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3292,9 +3261,7 @@ ssize_t vmci_qpair_dequev(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - &msg->msg_iter, msg_data_left(msg), - qp_memcpy_from_queue_iov, - true); + &msg->msg_iter, true); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3336,9 +3303,7 @@ ssize_t vmci_qpair_peekv(struct vmci_qp *qpair, result = qp_dequeue_locked(qpair->produce_q, qpair->consume_q, qpair->consume_q_size, - &msg->msg_iter, msg_data_left(msg), - qp_memcpy_from_queue_iov, - false); + &msg->msg_iter, false); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) -- cgit From d1038084415c413b3c11c536f28fc5571ed00153 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 14 Jul 2017 16:54:08 -0400 Subject: vmci: the same on the send side... Signed-off-by: Al Viro --- drivers/misc/vmw_vmci/vmci_queue_pair.c | 89 ++++++++------------------------- 1 file changed, 20 insertions(+), 69 deletions(-) diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c index 09314b991fdf..0339538c182d 100644 --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c @@ -129,20 +129,6 @@ * *_MEM state, and vice versa. */ -/* - * VMCIMemcpy{To,From}QueueFunc() prototypes. Functions of these - * types are passed around to enqueue and dequeue routines. Note that - * often the functions passed are simply wrappers around memcpy - * itself. - * - * Note: In order for the memcpy typedefs to be compatible with the VMKernel, - * there's an unused last parameter for the hosted side. In - * ESX, that parameter holds a buffer type. - */ -typedef int vmci_memcpy_to_queue_func(struct vmci_queue *queue, - u64 queue_offset, const void *src, - size_t src_offset, size_t size); - /* The Kernel specific component of the struct vmci_queue structure. */ struct vmci_queue_kern_if { struct mutex __mutex; /* Protects the queue. */ @@ -348,11 +334,10 @@ static void *qp_alloc_queue(u64 size, u32 flags) * by traversing the offset -> page translation structure for the queue. * Assumes that offset + size does not wrap around in the queue. */ -static int __qp_memcpy_to_queue(struct vmci_queue *queue, - u64 queue_offset, - const void *src, - size_t size, - bool is_iovec) +static int qp_memcpy_to_queue_iter(struct vmci_queue *queue, + u64 queue_offset, + struct iov_iter *from, + size_t size) { struct vmci_queue_kern_if *kernel_if = queue->kernel_if; size_t bytes_copied = 0; @@ -377,23 +362,12 @@ static int __qp_memcpy_to_queue(struct vmci_queue *queue, else to_copy = size - bytes_copied; - if (is_iovec) { - struct msghdr *msg = (struct msghdr *)src; - int err; - - /* The iovec will track bytes_copied internally. */ - err = memcpy_from_msg((u8 *)va + page_offset, - msg, to_copy); - if (err != 0) { - if (kernel_if->host) - kunmap(kernel_if->u.h.page[page_index]); - return VMCI_ERROR_INVALID_ARGS; - } - } else { - memcpy((u8 *)va + page_offset, - (u8 *)src + bytes_copied, to_copy); + if (!copy_from_iter_full((u8 *)va + page_offset, to_copy, + from)) { + if (kernel_if->host) + kunmap(kernel_if->u.h.page[page_index]); + return VMCI_ERROR_INVALID_ARGS; } - bytes_copied += to_copy; if (kernel_if->host) kunmap(kernel_if->u.h.page[page_index]); @@ -554,30 +528,6 @@ static int qp_populate_ppn_set(u8 *call_buf, const struct ppn_set *ppn_set) return VMCI_SUCCESS; } -static int qp_memcpy_to_queue(struct vmci_queue *queue, - u64 queue_offset, - const void *src, size_t src_offset, size_t size) -{ - return __qp_memcpy_to_queue(queue, queue_offset, - (u8 *)src + src_offset, size, false); -} - -/* - * Copies from a given iovec from a VMCI Queue. - */ -static int qp_memcpy_to_queue_iov(struct vmci_queue *queue, - u64 queue_offset, - const void *msg, - size_t src_offset, size_t size) -{ - - /* - * We ignore src_offset because src is really a struct iovec * and will - * maintain offset internally. - */ - return __qp_memcpy_to_queue(queue, queue_offset, msg, size, true); -} - /* * Allocates kernel VA space of specified size plus space for the queue * and kernel interface. This is different from the guest queue allocator, @@ -2590,12 +2540,11 @@ static bool qp_wait_for_ready_queue(struct vmci_qp *qpair) static ssize_t qp_enqueue_locked(struct vmci_queue *produce_q, struct vmci_queue *consume_q, const u64 produce_q_size, - const void *buf, - size_t buf_size, - vmci_memcpy_to_queue_func memcpy_to_queue) + struct iov_iter *from) { s64 free_space; u64 tail; + size_t buf_size = iov_iter_count(from); size_t written; ssize_t result; @@ -2615,15 +2564,15 @@ static ssize_t qp_enqueue_locked(struct vmci_queue *produce_q, written = (size_t) (free_space > buf_size ? buf_size : free_space); tail = vmci_q_header_producer_tail(produce_q->q_header); if (likely(tail + written < produce_q_size)) { - result = memcpy_to_queue(produce_q, tail, buf, 0, written); + result = qp_memcpy_to_queue_iter(produce_q, tail, from, written); } else { /* Tail pointer wraps around. */ const size_t tmp = (size_t) (produce_q_size - tail); - result = memcpy_to_queue(produce_q, tail, buf, 0, tmp); + result = qp_memcpy_to_queue_iter(produce_q, tail, from, tmp); if (result >= VMCI_SUCCESS) - result = memcpy_to_queue(produce_q, 0, buf, tmp, + result = qp_memcpy_to_queue_iter(produce_q, 0, from, written - tmp); } @@ -3078,18 +3027,21 @@ ssize_t vmci_qpair_enqueue(struct vmci_qp *qpair, int buf_type) { ssize_t result; + struct iov_iter from; + struct kvec v = {.iov_base = (void *)buf, .iov_len = buf_size}; if (!qpair || !buf) return VMCI_ERROR_INVALID_ARGS; + iov_iter_kvec(&from, WRITE | ITER_KVEC, &v, 1, buf_size); + qp_lock(qpair); do { result = qp_enqueue_locked(qpair->produce_q, qpair->consume_q, qpair->produce_q_size, - buf, buf_size, - qp_memcpy_to_queue); + &from); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) @@ -3219,8 +3171,7 @@ ssize_t vmci_qpair_enquev(struct vmci_qp *qpair, result = qp_enqueue_locked(qpair->produce_q, qpair->consume_q, qpair->produce_q_size, - msg, msg_data_left(msg), - qp_memcpy_to_queue_iov); + &msg->msg_iter); if (result == VMCI_ERROR_QUEUEPAIR_NOT_READY && !qp_wait_for_ready_queue(qpair)) -- cgit