Re: [PATCH v4 15/17] KVM: selftests: Add __vmwrite() and use it when initializing optional fields.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to convert all blind writes of optional VMCS fields to the non-asserting `__vmwrite()` in the common setup code, contradicting its stated intent and setting up a test crash.
--

commit 2bdd2a9b67ebe18b99a514555581198b78d1fab6
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add __vmwrite() and use it when initializing optional fields.

This commit introduces __vmwrite(), a non-asserting version of vmwrite(), to
allow writing optional VMCS fields in common setup code where success is not
guaranteed. It prepares for vmwrite() to unconditionally assert success in
subsequent patches.

> diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
> index 1a8515de42b06..b6bd9423e6768 100644
> --- a/tools/testing/selftests/kvm/lib/x86/vmx.c
> +++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
> @@ -252,7 +252,7 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
>  	u32 sec_exec_ctl = 0;
>  
>  	vmwrite(VIRTUAL_PROCESSOR_ID, 0);

[Severity: Medium]
Will this blindly write to the VIRTUAL_PROCESSOR_ID field using the asserting
vmwrite()?

Since VPID is an optional secondary control that may not exist on all
hardware, this could crash selftests with a VM-Fail assertion on unsupported
hardware once vmwrite() is modified to assert success. Could this be converted
to __vmwrite() similar to POSTED_INTR_NV?

> -	vmwrite(POSTED_INTR_NV, 0);
> +	__vmwrite(POSTED_INTR_NV, 0);
>  
>  	vmwrite(PIN_BASED_VM_EXEC_CONTROL, rdmsr(MSR_IA32_VMX_TRUE_PINBASED_CTLS));

[ ... ]

> @@ -356,8 +356,8 @@ static inline void init_vmcs_guest_state(void *rip, void *rsp)
>  	vmwrite(GUEST_GS_SELECTOR, vmreadz(HOST_GS_SELECTOR));
>  	vmwrite(GUEST_LDTR_SELECTOR, 0);
>  	vmwrite(GUEST_TR_SELECTOR, vmreadz(HOST_TR_SELECTOR));
> -	vmwrite(GUEST_INTR_STATUS, 0);
> -	vmwrite(GUEST_PML_INDEX, 0);
> +	__vmwrite(GUEST_INTR_STATUS, 0);
> +	__vmwrite(GUEST_PML_INDEX, 0);
>  
>  	vmwrite(VMCS_LINK_POINTER, -1ll);
>  	vmwrite(GUEST_IA32_DEBUGCTL, 0);

[Severity: Medium]
Will these writes to optional guest state fields cause test failures on CPUs
or nested hypervisors that lack these features?

Fields like GUEST_IA32_DEBUGCTL, GUEST_IA32_PAT, GUEST_IA32_EFER, and
GUEST_IA32_PERF_GLOBAL_CTRL are written unconditionally using vmwrite() here
in init_vmcs_guest_state(). It seems they will assert failure on unsupported
hardware. Should these also be switched to use __vmwrite() to avoid crashing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904002423.3154436-1-seanjc@google.com?part=15