Re: [PATCH 4/4] KVM: selftests: Add Hyper-V hypercall forwarding test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] KVM: selftests: missing volatile and XMM constraints in
  guest_code()
--

commit 10757ad97814a547a0e0ce57f22b5e41e1c14071
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: selftests: Add Hyper-V hypercall forwarding test

This commit adds a kselftest for KVM_CAP_HYPERV_FORWARD_HCALL covering feature
mask validation, memory-based and fast hypercall exits to userspace, XMM
register output, 64-bit result handling, and KVM_HYPERV_HCALL_UD fault
injection.

> diff --git a/tools/testing/selftests/kvm/x86/hyperv_forward_hcall.c b/tools/testing/selftests/kvm/x86/hyperv_forward_hcall.c
> new file mode 100644
> index 0000000000000..5afc99ed533c8
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/hyperv_forward_hcall.c
> @@ -0,0 +1,268 @@
[ ... ]
> +static void guest_code(gpa_t pgs_gpa)
> +{
> +	__sse128_u in, out;
> +	u64 status;
> +	u8 vector;
[ ... ]
> +	/*
> +	 * Fast form: the register parameters and XMM0 reach userspace, and
> +	 * the XMM1 value userspace writes reaches the guest.
> +	 */
> +	in.as_u64[0] = XMM_IN_MARKER;
> +	in.as_u64[1] = XMM_IN_MARKER + 1;
> +	out.as_u64[0] = 0;
> +	out.as_u64[1] = 0;
> +	write_sse_reg(0, &in.vec);
> +	write_sse_reg(1, &out.vec);
> +	vector = __hyperv_hypercall(HVCALL_TEST_CODE | HV_HYPERCALL_FAST_BIT,
> +				    FAST_PARAM0, FAST_PARAM1, &status);
> +	read_sse_reg(1, &out.vec);
> +	GUEST_ASSERT(!vector);
> +	GUEST_ASSERT_EQ(status, USERSPACE_STATUS);
> +	GUEST_ASSERT_EQ(out.as_u64[0], XMM_OUT_MARKER);
> +	GUEST_ASSERT_EQ(out.as_u64[1], XMM_OUT_MARKER + 1);
> +	GUEST_SYNC(STAGE_FAST);

[Severity: Low]
Can compiler instruction scheduling or XMM register allocation reorder
read_sse_reg() or clobber xmm0 across __hyperv_hypercall()?

x86 KVM selftests are built with -O2 -march=x86-64-v2 (with SSE enabled), and
read_sse_reg() and write_sse_reg() omit both volatile and XMM register
constraints:

tools/testing/selftests/kvm/include/x86/processor.h:read_sse_reg() {
	switch (reg) {
    ...
	case 1:
		asm("movdqa %%xmm1, %0" : "=m"(*data));
		break;
    ...
}

tools/testing/selftests/kvm/include/x86/processor.h:write_sse_reg() {
	switch (reg) {
	case 0:
		asm("movdqa %0, %%xmm0" : : "m"(*data));
		break;
    ...
}

Because read_sse_reg() has an output operand, no input operands, and no
volatile qualifier, the compiler treats it as a movable computation with no
side effects, and the memory clobber in __hyperv_hypercall() does not prevent
hoisting read_sse_reg(1, &out.vec) ahead of the hypercall since out is a
non-escaping local variable.  In addition, because neither write_sse_reg() nor
__hyperv_hypercall() declares XMM register operands or clobbers, the compiler
can use xmm0 when zero-initializing out after write_sse_reg(0, &in.vec),
causing spurious test assertion failures in handle_fast_hcall() or
guest_code().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111205836.4186584-1-dwmw2@infradead.org?part=4