Re: [PATCH v3 9/9] KVM: selftests: Add nested SVM DecodeAssists test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The selftest `test_synthesized_npf_linear_wrap` incorrectly asserts that 64-bit linear addresses wrap at 4GB in compatibility mode, cementing a preexisting KVM emulator bug.
--

--- Patch [9]: [PATCH v3 9/9] KVM: selftests: Add nested SVM DecodeAssists test ---
commit 90a4009aecfc1596a0c559275e2cf280935e0a29
Author: Tina Zhang <zhang_wei@open-hieco.net>

KVM: selftests: Add nested SVM DecodeAssists test

Add a nested SVM selftest for DecodeAssists. Verify that KVM exposes
the feature to L1 and provides architectural exit state for various
instructions and intercepts.

> diff --git a/tools/testing/selftests/kvm/x86/svm_nested_decode_assists_test.c b/tools/testing/selftests/kvm/x86/svm_nested_decode_assists_test.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/svm_nested_decode_assists_test.c
[ ... ]
> +static void test_synthesized_npf_linear_wrap(struct svm_test_data *svm,
> +					     gpa_t npf_gpa)
> +{
> +	struct vmcb *vmcb = svm->vmcb;
[ ... ]
> +	prepare_l2_for_vmrun(svm, setup_eip);
> +	run_guest(vmcb, svm->vmcb_gpa);
> +	GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_NPF);
> +	GUEST_ASSERT_EQ(vmcb->control.exit_info_2, npf_gpa);
> +	GUEST_ASSERT_EQ(vmcb->save.rip, outsb_eip);
> +	GUEST_ASSERT_EQ(vmcb->control.insn_len,
> +			sizeof(linear_wrap_insn_bytes));

[Severity: Medium]
Does this test enforce an architectural violation by requiring the KVM
emulator to wrap 64-bit linear addresses to 32 bits?

By setting up L2 in compatibility mode (CS.L=0) with a non-zero segment
base and an EIP that crosses the 4GB boundary, this asserts that KVM
successfully fetches all 15 instruction bytes from GVA 0. This appears to
cement a pre-existing KVM emulator bug rather than verifying correct
hardware behavior (where the correct 64-bit linear address would be
0x100000000).

> +	GUEST_ASSERT(!memcmp(vmcb->control.insn_bytes,
> +			     linear_wrap_insn_bytes,
> +			     sizeof(linear_wrap_insn_bytes)));
> +	vmcb->save.cs.attrib = cs_attrib;
> +	vmcb->save.cs.base = cs_base;
> +	vmcb->save.cs.limit = cs_limit;
> +}
[ ... ]
> +static void prepare_linear_wrap_outsb_code(struct kvm_vm *vm)
> +{
[ ... ]
> +	memcpy(&high_page[LINEAR_WRAP_OUTSB_OFFSET],
> +	       linear_wrap_insn_bytes,
> +	       PAGE_SIZE - LINEAR_WRAP_OUTSB_OFFSET);
> +	memcpy(low_page,
> +	       &linear_wrap_insn_bytes[PAGE_SIZE - LINEAR_WRAP_OUTSB_OFFSET],
> +	       sizeof(linear_wrap_insn_bytes) -
> +	       (PAGE_SIZE - LINEAR_WRAP_OUTSB_OFFSET));
> +
> +	virt_map(vm, LINEAR_WRAP_CODE_GVA, high_gpa, 1);
> +	virt_map(vm, 0, low_gpa, 1);

[Severity: Medium]
Is mapping GVA 0 here anticipating and validating the emulator's incorrect
32-bit truncation of the linear address?

If the emulator is ever fixed to comply with the AMD APM and use the full
64-bit address, wouldn't this test begin failing since it maps the
fallback wrapped address?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785411877.git.zhang_wei@open-hieco.net?part=9