[PATCH] kvm: selftests: ucall improvements

[PATCH] kvm: selftests: ucall improvements

[Andrew Jones]

Make sure we complete the I/O after determining we have a ucall,
which is I/O. Also allow the *uc parameter to optionally be NULL.
It's quite possible that a test case will only care about the
return value, like for example when looping on a check for
UCALL_DONE.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/lib/ucall.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/ucall.c b/tools/testing/selftests/kvm/lib/ucall.c
index a2ab38be2f47..5994923dfc1e 100644
--- a/tools/testing/selftests/kvm/lib/ucall.c
+++ b/tools/testing/selftests/kvm/lib/ucall.c
@@ -125,16 +125,16 @@ void ucall(uint64_t cmd, int nargs, ...)
 uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
 {
 	struct kvm_run *run = vcpu_state(vm, vcpu_id);
-
-	memset(uc, 0, sizeof(*uc));
+	struct ucall ucall = {};
+	bool got_ucall = false;
 
 #ifdef __x86_64__
 	if (ucall_type == UCALL_PIO && run->exit_reason == KVM_EXIT_IO &&
 	    run->io.port == UCALL_PIO_PORT) {
 		struct kvm_regs regs;
 		vcpu_regs_get(vm, vcpu_id, &regs);
-		memcpy(uc, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi), sizeof(*uc));
-		return uc->cmd;
+		memcpy(&ucall, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi), sizeof(ucall));
+		got_ucall = true;
 	}
 #endif
 	if (ucall_type == UCALL_MMIO && run->exit_reason == KVM_EXIT_MMIO &&
@@ -143,8 +143,15 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
 		TEST_ASSERT(run->mmio.is_write && run->mmio.len == 8,
 			    "Unexpected ucall exit mmio address access");
 		gva = *(vm_vaddr_t *)run->mmio.data;
-		memcpy(uc, addr_gva2hva(vm, gva), sizeof(*uc));
+		memcpy(&ucall, addr_gva2hva(vm, gva), sizeof(ucall));
+		got_ucall = true;
+	}
+
+	if (got_ucall) {
+		vcpu_run_complete_io(vm, vcpu_id);
+		if (uc)
+			memcpy(uc, &ucall, sizeof(ucall));
 	}
 
-	return uc->cmd;
+	return ucall.cmd;
 }
-- 
2.20.1

Re: [PATCH] kvm: selftests: ucall improvements

[Peter Xu]

On Thu, May 23, 2019 at 12:46:35PM +0200, Andrew Jones wrote:
> Make sure we complete the I/O after determining we have a ucall,
> which is I/O. Also allow the *uc parameter to optionally be NULL.
> It's quite possible that a test case will only care about the
> return value, like for example when looping on a check for
> UCALL_DONE.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Would it make more sense to include the other patch altogether to
cleanup the existing users of get_ucall() where uc can be NULL?

Thanks,

-- 
Peter Xu

Re: [PATCH] kvm: selftests: ucall improvements

[Andrew Jones]

On Mon, May 27, 2019 at 02:08:31PM +0800, Peter Xu wrote:
> On Thu, May 23, 2019 at 12:46:35PM +0200, Andrew Jones wrote:
> > Make sure we complete the I/O after determining we have a ucall,
> > which is I/O. Also allow the *uc parameter to optionally be NULL.
> > It's quite possible that a test case will only care about the
> > return value, like for example when looping on a check for
> > UCALL_DONE.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Would it make more sense to include the other patch altogether to
> cleanup the existing users of get_ucall() where uc can be NULL?
>

Yeah, I can do that. I'll send a v2 with the cleanup and your r-b.

Thanks,
drew