Re: [PATCH v8 8/8] 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:
- [Low] kvm: selftests: missing .global directive on inline assembly labels causes LTO link error
--

commit 08e87505d5c0bae297f7e3e768c2e12a61d10322
Author: Tina Zhang <zhang_wei@open-hieco.net>

KVM: selftests: Add nested SVM DecodeAssists test

This commit adds a nested SVM selftest to verify KVM's virtualization of
DecodeAssists. It covers instruction byte extraction for various instructions
and edge cases such as synthesized nested page faults and userspace-injected
page faults.

> 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
> index 0000000000000..aba91d8bb1a28
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/svm_nested_decode_assists_test.c
> @@ -0,0 +1,590 @@

[ ... ]

> +extern u8 l2_movsb_insn[];
> +
> +static void l2_movsb_code(void)
> +{
> +	asm volatile("mov %0, %%rsi\n\t"
> +		     "mov %1, %%rdi\n\t"
> +		     "l2_movsb_insn:\n\t"

[Severity: Low]
Is a .global (or .globl) directive needed for this assembly label? 

When building the selftests with Link-Time Optimization (LTO) enabled, the 
l2_movsb_code function and the functions referencing the extern l2_movsb_insn 
array (like test_synthesized_npf_after_hardware_npf) might be partitioned 
into different LTRANS units. 

Since this label lacks a global directive, it remains strictly local to its 
inline assembly block's partition. The linker therefore cannot resolve the 
extern declaration in other partitions, causing an undefined reference build 
error.

> +		     "movsb"
> +		     : : "r"(&mmio_source), "r"(&npf_target)
> +		     : "rsi", "rdi", "memory");
> +	GUEST_FAIL("L2 MOVSB did not cause a nested page fault");
> +}
> +
> +extern u8 l2_userspace_pf_insn[];
> +
> +static void l2_userspace_pf_code(void)
> +{
> +	asm volatile("movzbl (%0), %%eax\n\t"
> +		     "l2_userspace_pf_insn:\n\t"

[Severity: Low]
Does this label also need a .global directive to prevent LTO linkage failures, 
similar to the l2_movsb_insn label above?

> +		     "nop"
> +		     : : "r"(&mmio_source) : "rax", "memory");
> +	GUEST_FAIL("Userspace-injected #PF was not intercepted by L1");
> +}
> +

[ ... ]

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