Re: [PATCH 7/7] KVM: SVM: Enable save/restore of FRED MSRs

[Sean Christopherson <seanjc@google.com>]

On Wed, Aug 19, 2026, Shivansh Dhiman wrote:
> On 13-08-26 04:57, Sean Christopherson wrote:
> >> Would you prefer it handled in svm_recalc_fred_msr_intercepts() instead
> >> while setting intercepts? Or is there a better way to deal with this?
> > 
> > Take a hard dependency on an upcoming APM update that states the control area
> > is valid after shutdown, and rework KVM to not clobber control fields on shutdown
> > interception.
> 
> Sure, Sean. I'm thinking something like the diff below. It contains three
> parts:
> 
> 1. Clear the save area and offsets 60h, 61h, and 68h of the control area,
>    as per the APM.
> 
> 2. Zero the intercepts as well. The APM doesn't require it, but it seemed
>    worthwhile so that the new intercepts are computed on a clean slate
>    rather than on top of whatever survived the triple fault. Is that worth
>    doing?

My gut reaction is I don't see how this can possibly be safe or correct.  How
does the guest taking a series of faults have anything to do with what accesses
KVM needs to intercept?

> 3. For nested, same fields are copied to vmcb12 when a shutdown is intercepted
>    in L2. Clear them while copying so L1 gets a deterministic value
>    rather than whatever the hardware left behind. Does that seem like right
>    thing to do?

No, because that violates the rule of "don't make stuff up".  It could also be
actively detrimental to L1's ability to debug issues (e.g. if hardware leaves
breadcrumbs in the save area), and risks creating guest<=>host ABI that we don't
want to support.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 83de3456df708..38843b29b21ec 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1149,6 +1149,11 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
>         vmcb12->control.event_inj         = svm->nested.ctl.event_inj;
>         vmcb12->control.event_inj_err     = svm->nested.ctl.event_inj_err;
> 
> +       if (vmcb02->control.exit_code == SVM_EXIT_SHUTDOWN) {
> +               vmcb12->control.int_ctl &= ~GENMASK(15, 0);
> +               vmcb12->control.int_state = 0;
> +       }
> +
>         if (!kvm_pause_in_guest(vcpu->kvm)) {
>                 vmcb01->control.pause_filter_count = vmcb02->control.pause_filter_count;
>                 vmcb_mark_dirty(vmcb01, VMCB_INTERCEPTS);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 519249e75420d..62b4feb9c3909 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2090,7 +2090,11 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
>          * cannot be reinitialized, i.e. synthesizing INIT is futile.
>          */
>         if (!sev_es_guest(vcpu->kvm)) {
> -               clear_page(svm->vmcb);
> +               struct vmcb_control_area *control = &svm->vmcb->control;
> +               memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
> +               memset(control->intercepts, 0, sizeof(control->intercepts));
> +               control->int_ctl &= ~GENMASK(15, 0);
> +               control->int_state = 0;
>  #ifdef CONFIG_KVM_SMM
>                 if (is_smm(vcpu))
>                         kvm_smm_changed(vcpu, false);
> 
> 
> Cheers,
> Shivansh
>