Skip to contentSkip to conversation

kvm: selftests: ucall improvements

PATCH Superseded Andrew Jones 2 participants 3 messages May 23, 2019 – May 27, 2019

Subsystems (2)
KVM Kselftest

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Andrew Jones

author
#0 lore raw

PATCH 1/1 kvm: selftests: ucall improvements

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 <[email protected]>
---
 tools/testing/selftests/kvm/lib/ucall.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
tools/testing/selftests/kvm/lib/ucall.c +13 -6
index a2ab38be2f47..5994923dfc1e 100644
@@ -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;
 }
Signature · 1 line
2.20.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · 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 <[email protected]>
Reviewed-by Peter Xu <[email protected]>

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,

Signature · 1 line
Peter Xu
author
#2 lore raw
Quoted 7 lines · Peter Xu
Quoted 6 lines · 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.

Quoted 7 lines
Quoted 1 line
Signed-off-by Andrew Jones <[email protected]>
Reviewed-by Peter Xu <[email protected]>

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