[PATCH v2] KVM: selftests: Check result in hyperv_features.c test only for successful hypercalls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Commit cc5851c6be86 ("KVM: selftests: Use exception fixup for #UD/#GP
Hyper-V MSR/hcall tests") introduced a wrong guest assert in guest_hcall().
It is not checking the successful hypercall results and only checks the result
when a fault happens.

  GUEST_ASSERT_2(!hcall->ud_expected || res == hcall->expect,
                 hcall->expect, res);

Correct the assertion by only checking results of the successful
hypercalls.

This issue was observed when this test started failing after building it
in Clang. Above guest assert statement fails because "res" is not equal
to "hcall->expect" when "hcall->ud_expected" is true. "res" gets some
garbage value in Clang from the RAX register. In GCC, RAX is 0 because
it using RAX for @output_address in the asm statement and resetting it
to 0 before using it as output operand in the same asm statement. Clang
is not using RAX for @output_address.

Load RAX with some default input value so that the compiler cannot
modify it or use it for anything else. This makes sure that KVM is
correctly clearing up return value on successful hypercall and compiler cannot
generate any false positive.

Fixes: cc5851c6be86 ("KVM: selftests: Use exception fixup for #UD/#GP Hyper-V MSR/hcall tests")
Signed-off-by: Vipin Sharma <vipinsh@xxxxxxxxxx>
Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>

---

Jim's Reviewed-by is only for the code change and not shortlog message
of v1. Also, there is one change in asm which was not present in v1 and
not reviewed by Jim. But I am writing his name here so that it is not missed
when patch is merged.

v2:
- Updated the shortlog message.
- Using RAX register in hypercall asm as input operand also and
  initializing it with -EFAULT

v1:
https://lore.kernel.org/lkml/20220921231151.2321058-1-vipinsh@xxxxxxxxxx/

 tools/testing/selftests/kvm/x86_64/hyperv_features.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index 79ab0152d281..4d55e038c2d7 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -26,7 +26,8 @@ static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
 		     : "=a" (*hv_status),
 		       "+c" (control), "+d" (input_address),
 		       KVM_ASM_SAFE_OUTPUTS(vector)
-		     : [output_address] "r"(output_address)
+		     : [output_address] "r"(output_address),
+		       "a" (-EFAULT)
 		     : "cc", "memory", "r8", KVM_ASM_SAFE_CLOBBERS);
 	return vector;
 }
@@ -81,13 +82,13 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
 	}
 
 	vector = hypercall(hcall->control, input, output, &res);
-	if (hcall->ud_expected)
+	if (hcall->ud_expected) {
 		GUEST_ASSERT_2(vector == UD_VECTOR, hcall->control, vector);
-	else
+	} else {
 		GUEST_ASSERT_2(!vector, hcall->control, vector);
+		GUEST_ASSERT_2(res == hcall->expect, hcall->expect, res);
+	}
 
-	GUEST_ASSERT_2(!hcall->ud_expected || res == hcall->expect,
-			hcall->expect, res);
 	GUEST_DONE();
 }
 
-- 
2.37.3.968.ga6b4b080e4-goog




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux