Re: [PATCH v16 36/51] KVM: nSVM: Save/load CET Shadow Stack state to/from vmcb12/vmcb02

Yosry Ahmed
2025-10-28T22:23:11Z

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.

> 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
>