Re: [PATCH v3 05/10] KVM: selftests: Reuse GPR switching logic for nVMX

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced VMX_SWITCH_GPRS_ASM logic makes vmlaunch and vmresume thread-unsafe, corrupting L1 host and L2 guest register state in multi-vCPU nested tests.
--

commit f53344a36faa652dd7b5c7d1e749a4bffbc5242b
Author: Yosry Ahmed <yosry@kernel.org>

KVM: selftests: Reuse GPR switching logic for nVMX

This commit introduces VMX_SWITCH_GPRS_ASM to reuse the GPR switching logic
from SVM for nVMX. It replaces the push/pop of registers with
VMX_SWITCH_GPRS_ASM during vmlaunch and vmresume operations for both standard
and enlightened VMCS.

> diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
> index 4bcfd60e3aecb..a808dc21c9f21 100644
> --- a/tools/testing/selftests/kvm/include/x86/vmx.h
> +++ b/tools/testing/selftests/kvm/include/x86/vmx.h
[ ... ]
> @@ -373,34 +387,23 @@ static inline int vmlaunch(void)
>  	if (enable_evmcs)
>  		return evmcs_vmlaunch();
>  
> -	__asm__ __volatile__("push %%rbp;"
> -			     "push %%rcx;"
> -			     "push %%rdx;"
> -			     "push %%rsi;"
> -			     "push %%rdi;"
> -			     "push $0;"
> +	__asm__ __volatile__("push $0;"
>  			     "vmwrite %%rsp, %[host_rsp];"
>  			     "lea 1f(%%rip), %%rax;"
>  			     "vmwrite %%rax, %[host_rip];"
> +			     VMX_SWITCH_GPRS_ASM
>  			     "vmlaunch;"

[Severity: Medium]
Will this replacement make vmlaunch() thread-unsafe in multi-vCPU nested
tests?

Since VMX_SWITCH_GPRS_ASM uses GUEST_SWITCH_GPR_ASM which operates on a
single shared guest_regs array in L1 memory, concurrent executions by
multiple vCPUs (e.g., via memstress_l2_guest_entry) could clobber the saved
host state and L2 initial state for all but one vCPU.

The previous push/pop sequence was thread-safe since it used the stack.
Does this same thread-safety issue apply to vmresume() and the evmcs variants
that also adopted this macro?

>  			     "incq (%%rsp);"
> -			     "1: pop %%rax;"
> -			     "pop %%rdi;"
> -			     "pop %%rsi;"
> -			     "pop %%rdx;"
> -			     "pop %%rcx;"
> -			     "pop %%rbp;"
> +			     "1: ;"
> +			     VMX_SWITCH_GPRS_ASM
> +			     "pop %%rax;"
>  			     : [ret]"=&a"(ret)
>  			     : [host_rsp]"r"((u64)HOST_RSP),
>  			       [host_rip]"r"((u64)HOST_RIP)
> -			     : "memory", "cc", "rbx", "r8", "r9", "r10",
> -			       "r11", "r12", "r13", "r14", "r15");
> +			     : "memory", "cc");
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629183746.699840-1-yosry@kernel.org?part=5