From: Zhao Liu <zhao1.liu@intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
Shuah Khan <shuah@kernel.org>, Chao Gao <chao.gao@intel.com>,
Xin Li <xin@zytor.com>, Sohil Mehta <sohil.mehta@intel.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org, Zhao Liu <zhao1.liu@intel.com>
Subject: Re: [RFC 1/4] KVM: nVMX: Don't copy L2's CET state to L1 if VM-entry didn't load it
Date: Mon, 7 Sep 2026 20:47:46 +0800 [thread overview]
Message-ID: <ap6yciu1m7tz+NGv@intel.com> (raw)
In-Reply-To: <apr02qOMhsZnXIlV@google.com>
Hi Sean,
Thanks for your feedback!
On Fri, Sep 04, 2026 at 09:42:02AM -0700, Sean Christopherson wrote:
> Date: Fri, 4 Sep 2026 09:42:02 -0700
> From: Sean Christopherson <seanjc@google.com>
> Subject: Re: [RFC 1/4] KVM: nVMX: Don't copy L2's CET state to L1 if
> VM-entry didn't load it
>
> On Fri, Sep 04, 2026, Zhao Liu wrote:
> > On a nested VM-exit that disables VM_EXIT_LOAD_CET_STATE, only copy L2's
> > CET state from vmcs12 to vmcs01 if VM-entry really loaded that state,
> > i.e. don't copy when VM-entry fails before loading guest state.
> >
> > The state, that L1 should see after a L2 VM-exit, depends on three
> > things: the VM-exit load (host state) control, whether VM-entry loaded
> > L2's state, and whether L2 ran.
>
> No, it depends on four things. The three things you listed, plus uarch-specific
> ordering of checks and loads of guest state. The SDM says:
>
> the following operations take place concurrently:
>
> (1) the guest-state area of the VMCS is checked to ensure that, after the
> VM entry completes, the state of the logical processor is consistent
> with IA-32 and Intel 64 architectures;
> (2) processor state is loaded from the guest-state area or as specified by
> the VM-entry control fields; and (3) address-range monitoring is cleared.
>
> Because the checking and the loading occur concurrently, a failure may be
> discovered only after some state has been loaded. For this reason, the logical
> processor responds to such failures by loading state from the host-state area,
> as it would for a VM exit.
>
> So KVM is well within its rights to load vmcs01 state from vmcs12 even on VM-Exit
> due to a failed VM-Entry. More at the very bottom (below the first diff).
Yes, I agree. I think the specific way KVM handles guest state can also be
considered "uarch-specific" and is not part of the ABI guarantee.
> This works, but IMO is unnecessarily convoluted. KVM doesn't need to manually
> query vmcs12 entry controls, we can and should instead call sync_vmcs02_to_vmcs12()
> in the failed VM-Entry path if vmcs02 has been prepared with vmcs12 state. Then
> the only thing that needs to be communicated to load_vmcs12_host_state() is
> whether or not vmcs02 was prepared. This would make KVM consistent with how it
> handles guest state on failed VM-Entry VM-Exits that occur because of hardware's
> consistency checks (KVM only validates a subset of guest state).
>
> So I'm fairly certain it's just the below change (I also tweaked the comment about
> CET state because it's not at all obvious why vmcs12 would hold the correct state).
I tested this solution, and it passed the selftest in patch 4. Thanks!
But when I think more deeply about what the sync_vmcs02_to_vmcs12()
function actually does, I think there are still two points worth
discussing:
1. We call sync_vmcs02_to_vmcs12() along the vm-entry failure path; for
CET, this effectively overwrites the CET guest in vmcs12 with the
expected L2 CET state.
Per SDM 29.8, regarding VM-entry failure handling - it states:
Although this process resembles that of a VM exit, many steps taken
during a VM exit do not occur for these VM-entry failures:
• Most VM-exit information fields are not updated (see step 1 above).
• The valid bit in the injected-event identification field is not cleared.
-> • The guest-state area is not modified.
• No MSRs are saved into the VM-exit MSR-store area.
The key point here I want to refer is that the guest state should
remain unchanged. And considering this case, when VM_ENTRY_LOAD_CET_STATE
is cleared, KVM tries to load L1's CET state to vmcs02 (vmcs01->vmcs02),
but sync_vmcs02_to_vmcs12() loads vmcs02 CET states back to vmcs12
(vmcs02->vmcs12), which changes the guest state and such vmcs12 guest
state is visible to L1 (so, the whole states transition path is:
vmcs01->vmcs02->vmcs12, the vmcs12 is changed!).
2. I feel sync_vmcs02_to_vmcs12() might be doing too much. One issue is
it updates vmcs12->guest_activity_state based on vcpu->arch.mp_state,
However, the "correct" vcpu->arch.mp_state is set based on
vmcs12->guest_activity_state only after the VM-entry succeeds (see
nested_vmx_run). Therefore, in an extreme case (just code reading, not
tested yet), GUEST_ACTIVITY_HLT might be set to guest_activity_state,
but because the vm-entry fails, the value of vcpu->arch.mp_state is
not updated, so sync_vmcs02_to_vmcs12() re-calculates an incorrect
guest_activity_state.
Another issue is about vmcs12->guest_linear_address. Per SDM 29.8
again (the 1st point I listed above), guest_linear_address, as the
VM-exit information field, is not necessary to be updated.
So, in short, I think sync_vmcs02_to_vmcs12() updates too much state,
and it seems we shouldn't directly modify the guest state visible to L1
(i.e., the guest state in vmcs12).
> if (from_vmentry) {
> failed_index = nested_vmx_load_msr(vcpu,
> vmcs12->vm_entry_msr_load_addr,
> @@ -3758,6 +3762,9 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
> * 26.7 "VM-entry failures during or after loading guest state".
> */
> vmentry_fail_vmexit_guest_mode:
> + if (prepared_vmcs02)
> + sync_vmcs02_to_vmcs12(vcpu, vmcs12);
Based on the two points mentioned above, does Option B (which I described
in my cover letter) seem more reasonable? Instead of updatding almost all
states to vmcs12, we can re-use pre_enter_* states:
vmentry_fail_vmexit_guest_mode:
if (prepared_vmcs02)
vmcs_read_cet_state(vcpu, &vmx->nested.pre_vmenter_s_cet,
&vmx->nested.pre_vmenter_ssp,
&vmx->nested.pre_vmenter_ssp_tbl);
...
> -static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
> - struct vmcs12 *vmcs12)
> +static void load_vmcs12_host_state(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
> + bool prepared_vmcs02)
> {
> enum vm_entry_failure_code ignored;
> struct kvm_segment seg;
> @@ -4854,14 +4861,15 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
> vmcs_write64(GUEST_BNDCFGS, 0);
>
> /*
> - * Load CET state from host state if VM_EXIT_LOAD_CET_STATE is set.
> - * otherwise CET state should be retained across VM-exit, i.e.,
> - * guest values should be propagated from vmcs12 to vmcs01.
> + * If CET state should be retained across VM-exit, i.e. isn't loaded
> + * from host state fields, and vmcs02 was prepared with guest state and
> + * thus synchronized back to vmcs12 (CET state is unconditionally saved
> + * on VM-Exit), then propagate the guest's values from vmcs12 to vmcs01.
> */
> if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_CET_STATE)
> vmcs_write_cet_state(vcpu, vmcs12->host_s_cet, vmcs12->host_ssp,
> vmcs12->host_ssp_tbl);
> - else
> + else if (prepared_vmcs02)
> vmcs_write_cet_state(vcpu, vmcs12->guest_s_cet, vmcs12->guest_ssp,
> vmcs12->guest_ssp_tbl);
then update L1's state based on pre_vmenter_*, and keep vmcs12 guest
states upchanged:
else if (prepared_vmcs02)
vmcs_write_cet_state(vcpu, vmx->nested.pre_vmenter_s_cet,
vmx->nested.pre_vmenter_ssp,
vmx->nested.pre_vmenter_ssp_tbl);
In addition, in this way, pre_vmenter_* should also be synchronized on
the normal exit path with sync_vmcs02_to_vmcs12().
However, I’m still hesitant because this intermediate state seems too
fragmented. But considering that vmcs12 is L1 visible — and shouldn’t
be modified arbitrarily — maybe an intermediate state like this is
necessary?
Or maybe we should just go back to my original patch, i.e., Option A,
which eliminates the intermediate state.
> As for nitpicking the SDM, KVM doesn't *need* to wait until prepare_vmcs02()
> completes cleanly, KVM just needs to guarantee that vmcs12 holds the correct state
> if L2 state is loaded from vmcs12 on VM-Exit. Because even on failure,
> prepare_vmcs02() has already loaded (most) guest state into vmcs02. So we could
> sync vmcs02=>vmcs12 on any failure after switching to vmcs02, if we adjusted
> prepare_vmcs02() to fully prepare vmcs02 before do its final consistency checks.
> I.e. we could do the below on top.
>
> However, as much as I want to be pedantic on this point, I don't think we should
> actually do the below. I combed through the flows and can't find anything that
> would result in loading the wrong L1 state if KVM mostly prepares vmcs02 but
> doesn't do sync_vmcs02_to_vmcs12(). And marking vmcs02 as prepared if and only
> if it's fully prepared is much more obviously correct.
Yes, I think both approaches - whether to wait until prepare_vmcs02()
completes cleanly or not - are consistent with the SDM; they can be
viewed as differences in the KVM uarch implementation :-) What the user
sees is the hardware state and the guest VMCS state as presented under
a given load control combination.
Thanks,
Zhao
next prev parent reply other threads:[~2026-09-07 12:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 2:31 [RFC 0/4] KVM: nVMX: Fix guest (CET) state handling on VM-entry failure Zhao Liu
2026-09-04 2:31 ` [RFC 1/4] KVM: nVMX: Don't copy L2's CET state to L1 if VM-entry didn't load it Zhao Liu
2026-09-04 2:49 ` sashiko-bot
2026-09-04 16:42 ` Sean Christopherson
2026-09-07 12:47 ` Zhao Liu [this message]
2026-09-04 2:31 ` [RFC 2/4] KVM: selftests: Synchronize and update VMCS controls Zhao Liu
2026-09-04 2:31 ` [RFC 3/4] KVM: selftests: Synchronize and update VMCS encodings Zhao Liu
2026-09-04 2:31 ` [RFC 4/4] KVM: selftests: Test VM-entry failure handling for nested VM Zhao Liu
2026-09-04 2:50 ` sashiko-bot
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=ap6yciu1m7tz+NGv@intel.com \
--to=zhao1.liu@intel.com \
--cc=bp@alien8.de \
--cc=chao.gao@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=shuah@kernel.org \
--cc=sohil.mehta@intel.com \
--cc=tglx@kernel.org \
--cc=x86@kernel.org \
--cc=xin@zytor.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.