Re: [PATCH v2 06/11] KVM: SVM: add wrappers to enable/disable IRET interception
From: Sean Christopherson
Date: Tue Jan 31 2023 - 16:07:56 EST
On Thu, Dec 08, 2022, Maxim Levitsky wrote:
> On Thu, 2022-12-08 at 17:39 +0530, Santosh Shukla wrote:
> >
> > On 12/6/2022 5:44 PM, Maxim Levitsky wrote:
> > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > > > > index 512b2aa21137e2..cfed6ab29c839a 100644
> > > > > --- a/arch/x86/kvm/svm/svm.c
> > > > > +++ b/arch/x86/kvm/svm/svm.c
> > > > > @@ -2468,16 +2468,29 @@ static int task_switch_interception(struct kvm_vcpu *vcpu)
> > > > > has_error_code, error_code);
> > > > > }
> > > > >
> > > > > +static void svm_disable_iret_interception(struct vcpu_svm *svm)
> > > > > +{
> > > > > + if (!sev_es_guest(svm->vcpu.kvm))
> > > > > + svm_clr_intercept(svm, INTERCEPT_IRET);
> > > > > +}
> > > > > +
> > > > > +static void svm_enable_iret_interception(struct vcpu_svm *svm)
> > > > > +{
> > > > > + if (!sev_es_guest(svm->vcpu.kvm))
> > > > > + svm_set_intercept(svm, INTERCEPT_IRET);
> > > > > +}
> > > > > +
> > > >
> > > > nits:
> > > > s/_iret_interception / _iret_intercept
> > > > does that make sense?
> > >
> > > Makes sense.
I would rather go with svm_{clr,set}_iret_intercept(). I don't particularly like
the SVM naming scheme, but I really dislike inconsistent naming. If we want to
clean up naming, I would love unify VMX and SVM nomenclature for things like this.
> > > I can also move this to svm.h near the svm_set_intercept(), I think
> > > it better a better place for this function there if no objections.
> > >
> > I think current approach is fine since function used in svm.c only. but I have
> > no strong opinion on moving to svm.h either ways.
>
> I also think so, just noticed something in case there are any objections.
My vote is to keep it in svm.c unless we anticipate usage outside of svm.h. Keeping
the implementation close to the usage makes it easer to understand what's going on,
especially for something like this where there's a bit of "hidden" logic for SEV-ES.