Re: [PATCH v2 1/6] KVM/x86: Change comment before KVM_MSR_RET_* defines

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The commit message and comment change incorrectly dictate the use of negative errno values instead of `1` for MSR emulation failures, which misunderstands KVM's return code semantics and causes fatal VM exits.
--

commit 71177dccfb6890950255b0c75cdd52620e26871f
Author: Juergen Gross <jgross@suse.com>

KVM/x86: Change comment before KVM_MSR_RET_* defines

This commit updates the documentation and comments for MSR emulation return
codes in KVM. It advises replacing the return value "1" with negative errno
values for error cases.

> Replace it with a statement that it is fine to use negative errno
> values, 0, and the special MSR emulation return values defined.
>
> Don't mention the value "1", as current cases where MSR emulation is
> returning "1" should be modified to return a negative errno value
> instead.

[Severity: Critical]
Does replacing "1" with negative errno values cause fatal VM exits instead
of injecting a #GP into the guest?

Looking at how KVM handles MSR emulation, a return value of 1 (or
KVM_MSR_RET_UNSUPPORTED) allows the code to fall through to
complete_emulated_msr(), which safely injects a #GP into the guest and
resumes the vcpu_run loop.

If a negative value is returned instead, __kvm_emulate_wrmsr() catches it
and returns immediately:

__kvm_emulate_wrmsr() {
        ...
        if (r < 0)
                return r;
        return complete_emulated_msr(vcpu, false);
}

This negative value propagates up through the exit handlers to vcpu_run(),
where it breaks the guest execution loop:

vcpu_run() {
        ...
        if (r <= 0)
                break;
        ...
}

This returns the negative errno directly to userspace, which typically
causes the VMM (like QEMU) to abort the VM. 

Since guest OSes routinely probe MSRs and expect a #GP for unsupported
registers, will changing these to negative errno values cause VMs to crash
during normal operation?

> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 38a905fa86de2..5ca2f3d44092c 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -675,8 +675,8 @@ enum kvm_msr_access {
>  /*
>   * Internal error codes that are used to indicate that MSR emulation encountered
>   * an error that should result in #GP in the guest, unless userspace handles it.
> - * Note, '1', '0', and negative numbers are off limits, as they are used by KVM
> - * as part of KVM's lightly documented internal KVM_RUN return codes.
> + * Other than these internal error codes negative errno values can be used for
> + * error cases, or 0 for success.
>   *
>   * UNSUPPORTED	- The MSR isn't supported, either because it is completely
>   *		  unknown to KVM, or because the MSR should not exist according

[Severity: Critical]
Is this comment update accurate regarding KVM's return code semantics?

As noted above, negative numbers bypass the #GP injection logic and are
treated as fatal errors by the vcpu_run loop. The original comment appeared
to correctly warn that negative values are part of KVM's internal KVM_RUN
return codes and should not be returned by MSR handlers directly.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528113605.267111-1-jgross@suse.com?part=1