Re: [PATCH 2/5] KVM: SVM: check validity of VMCB when returning from SMM

[Paolo Bonzini]

Il mar 10 mar 2026, 22:45 Sean Christopherson <seanjc@xxxxxxxxxx> ha scritto:
>
> On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > ---
> > >  arch/x86/kvm/svm/nested.c | 12 ++++++++++--
> > >  arch/x86/kvm/svm/svm.c    |  4 ++++
> > >  arch/x86/kvm/svm/svm.h    |  1 +
> > >  3 files changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > index 7b61124051a7..de9906adb73b 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -419,6 +419,15 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
> > >         return __nested_vmcb_check_controls(vcpu, ctl);
> > >  }
> > >
> > > +int nested_svm_check_cached_vmcb12(struct kvm_vcpu *vcpu)
> > > +{
> > > +       if (!nested_vmcb_check_save(vcpu) ||
> > > +           !nested_vmcb_check_controls(vcpu))
> > > +               return -EINVAL;
> > > +
> > > +       return 0;
> > > +}
> >
> > Nit: if we make this a boolean we could just do:
> >
> > bool nested_svm_check_cached_vmcb12(struct kvm_vcpu *vcpu)
> > {
> >        return nested_vmcb_check_save(vcpu) && nested_vmcb_check_controls(vcpu);
>
> I don't care one way or the other for this particular patch, but once the dust
> settles on nSVM (assuming it ever does) I do think we should align the "nested
> check" return types across nVMX and nSVM (which is likely why Paolo ended up with
> the above; I requested using -EINVAL for the nVMXx) patch.

I was indeed aiming for more similar code between the two. The last
few nSVM shakedowns prior to Yosry's (nested_run_pending/live
migration, vmcb01/02 split) already took some inspiration from nVMX
code and naming, so most of the low hanging fruit is gone and I didn't
want to actually make things worse...

Paolo


> My fairly strong preference is to use 0/-errno as "return -EINVAL" is more
> obviously an error than "return true".  But we can bikeshed later :-)
>