From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Jim Mattson <jmattson@google.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/11] KVM: nSVM: Add missing consistency check for event_inj
Date: Wed, 5 Nov 2025 10:48:28 -0800 [thread overview]
Message-ID: <aQub_AbP6l6BJlB2@google.com> (raw)
In-Reply-To: <20251104195949.3528411-4-yosry.ahmed@linux.dev>
On Tue, Nov 04, 2025, Yosry Ahmed wrote:
> According to the APM Volume #2, 15.20 (24593—Rev. 3.42—March 2024):
>
> VMRUN exits with VMEXIT_INVALID error code if either:
> • Reserved values of TYPE have been specified, or
> • TYPE = 3 (exception) has been specified with a vector that does not
> correspond to an exception (this includes vector 2, which is an NMI,
> not an exception).
>
> Add the missing consistency checks to KVM. For the second point, inject
> VMEXIT_INVALID if the vector is anything but the vectors defined by the
> APM for exceptions. Reserved vectors are also considered invalid, which
> matches the HW behavior.
Ugh. Strictly speaking, that means KVM needs to match the capabilities of the
virtual CPU. E.g. if the virtual CPU predates SEV-ES, then #VC should be reserved
from the guest's perspective.
> Vector 9 (i.e. #CSO) is considered invalid because it is reserved on modern
> CPUs, and according to LLMs no CPUs exist supporting SVM and producing #CSOs.
>
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
> arch/x86/include/asm/svm.h | 5 +++++
> arch/x86/kvm/svm/nested.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index e69b6d0dedcf0..3a9441a8954f3 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -633,6 +633,11 @@ static inline void __unused_size_checks(void)
> #define SVM_EVTINJ_VALID (1 << 31)
> #define SVM_EVTINJ_VALID_ERR (1 << 11)
>
> +/* Only valid exceptions (and not NMIs) are allowed for SVM_EVTINJ_TYPE_EXEPT */
> +#define SVM_EVNTINJ_INVALID_EXEPTS (NMI_VECTOR | BIT_ULL(9) | BIT_ULL(15) | \
> + BIT_ULL(20) | GENMASK_ULL(27, 22) | \
> + BIT_ULL(31))
As above, hardcoding this won't work. E.g. if a VM is migrated from a CPU where
vector X is reserved to a CPU where vector X is valid, then the VM will observe
a change in behavior.
Even if we're ok being overly permissive today (e.g. by taking an erratum), this
will create problems in the future when one of the reserved vectors is defined,
at which point we'll end up changing guest-visible behavior (and will have to
take another erratum, or maybe define the erratum to be that KVM straight up
doesn't enforce this correctly?)
And if we do throw in the towel and don't try to enforce this, we'll still want
a safeguard against this becoming stale, e.g. when KVM adds support for new
feature XYZ that comes with a new vector.
Off the cuff, the best idea I have is to define the positive set of vectors
somewhere common with a static assert, and then invert that. E.g. maybe something
shared with kvm_trace_sym_exc()?
next prev parent reply other threads:[~2025-11-05 18:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-04 19:59 [PATCH 00/11] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
2025-11-04 19:59 ` [PATCH 01/11] KVM: nSVM: Fix consistency checks for NP_ENABLE Yosry Ahmed
2025-11-05 18:52 ` Yosry Ahmed
2025-11-04 19:59 ` [PATCH 02/11] KVM: nSVM: Add missing consistency check for EFER, CR0, CR4, and CS Yosry Ahmed
2025-11-04 19:59 ` [PATCH 03/11] KVM: nSVM: Add missing consistency check for event_inj Yosry Ahmed
2025-11-05 18:48 ` Sean Christopherson [this message]
2025-11-05 19:29 ` Yosry Ahmed
2025-11-06 1:17 ` Sean Christopherson
2025-11-08 2:29 ` Yosry Ahmed
2025-11-04 19:59 ` [PATCH 04/11] KVM: SVM: Rename vmcb->nested_ctl to vmcb->misc_ctl Yosry Ahmed
2025-11-04 19:59 ` [PATCH 05/11] KVM: SVM: Rename vmcb->virt_ext to vmcb->misc_ctl2 Yosry Ahmed
2025-11-04 19:59 ` [PATCH 06/11] KVM: SVM: switch svm_copy_lbrs() to a macro Yosry Ahmed
2025-11-04 19:59 ` [PATCH 07/11] KVM: nSVM: Cache all used fields from VMCB12 Yosry Ahmed
2025-11-04 19:59 ` [PATCH 08/11] KVM: nSVM: Restrict mapping VMCB12 on nested VMRUN Yosry Ahmed
2025-11-04 19:59 ` [PATCH 09/11] KVM: nSVM: Simplify nested_svm_vmrun() Yosry Ahmed
2025-11-04 19:59 ` [PATCH 10/11] KVM: nSVM: Sanitize control fields copied from VMCB12 Yosry Ahmed
2025-11-04 19:59 ` [PATCH 11/11] KVM: nSVM: Only copy NP_ENABLE from VMCB01's nested_ctl Yosry Ahmed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aQub_AbP6l6BJlB2@google.com \
--to=seanjc@google.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=yosry.ahmed@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).