On Tue, Oct 28, 2025 at 10:23:02PM +0000, Yosry Ahmed wrote:
> On Fri, Sep 19, 2025 at 03:32:43PM -0700, Sean Christopherson wrote:
> > Transfer the three CET Shadow Stack VMCB fields (S_CET, ISST_ADDR, and
> > SSP) on VMRUN, #VMEXIT, and loading nested state (saving nested state
> > simply copies the entire save area). SVM doesn't provide a way to
> > disallow L1 from enabling Shadow Stacks for L2, i.e. KVM *must* provide
> > nested support before advertising SHSTK to userspace.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kvm/svm/nested.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 826473f2d7c7..a6443feab252 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -636,6 +636,14 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> > vmcb_mark_dirty(vmcb02, VMCB_DT);
> > }
> >
> > + if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK) &&
> > + (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_CET)))) {
> > + vmcb02->save.s_cet = vmcb12->save.s_cet;
> > + vmcb02->save.isst_addr = vmcb12->save.isst_addr;
> > + vmcb02->save.ssp = vmcb12->save.ssp;
> > + vmcb_mark_dirty(vmcb02, VMCB_CET);
> > + }
> > +
>
> According to the APM, there are some consistency checks that should be
> done on CET related fields in the VMCB12. Specifically from
> "Canonicalization and Consistency Checks. " in 15.5.1 in the APM Volume
> 2 (24593—Rev. 3.42—March 2024):
>
> • Any reserved bit is set in S_CET
> • CR4.CET=1 when CR0.WP=0
> • CR4.CET=1 and U_CET.SS=1 when EFLAGS.VM=1
> • Any reserved bit set in U_CET (SEV-ES only):
> - VMRUN results in VMEXIT(INVALID)
> - VMEXIT forces reserved bits to 0
>
> Most consistency checks are done in __nested_vmcb_check_save(), but it
> only operates on the cached save area, which does not have everything
> you need. You'll probably need to add the needed fields to the cached
> save area, or move the consistency checks elsewhere.
>
> Related to this, I am working on patches to copy everything we use from
> vmcb12->save to the cache area to minimize directly accessing vmcb12
> from the guest memory as much as possible. So I already intend to add
> other fields to the cached save area.
>
> There's also a couple of other missing consistency checks that I will
> send patches for, which also need fields currently not in the cached
> save area.
I don't really care that much, but I think this fell through the cracks.
Regarding the cached save area, the series I was talking about is
already out [*], and I am preparing to send a newer version. It puts the
fields used here in the cache, so it should be straightforward to add
the consistency checks on top of it.
[*]https://lore.kernel.org/kvm/20251110222922.613224-1-yosry.ahmed@linux.dev/
>
> > kvm_set_rflags(vcpu, vmcb12->save.rflags | X86_EFLAGS_FIXED);
> >
> > svm_set_efer(vcpu, svm->nested.save.efer);
> > @@ -1044,6 +1052,12 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
> > to_save->rsp = from_save->rsp;
> > to_save->rip = from_save->rip;
> > to_save->cpl = 0;
> > +
> > + if (kvm_cpu_cap_has(X86_FEATURE_SHSTK)) {
> > + to_save->s_cet = from_save->s_cet;
> > + to_save->isst_addr = from_save->isst_addr;
> > + to_save->ssp = from_save->ssp;
> > + }
> > }
> >
> > void svm_copy_vmloadsave_state(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
> > @@ -1111,6 +1125,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> > vmcb12->save.dr6 = svm->vcpu.arch.dr6;
> > vmcb12->save.cpl = vmcb02->save.cpl;
> >
> > + if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK)) {
> > + vmcb12->save.s_cet = vmcb02->save.s_cet;
> > + vmcb12->save.isst_addr = vmcb02->save.isst_addr;
> > + vmcb12->save.ssp = vmcb02->save.ssp;
> > + }
> > +
> > vmcb12->control.int_state = vmcb02->control.int_state;
> > vmcb12->control.exit_code = vmcb02->control.exit_code;
> > vmcb12->control.exit_code_hi = vmcb02->control.exit_code_hi;
> > --
> > 2.51.0.470.ga7dc726c21-goog
> >