linux-kernel - Re: [PATCH v2 06/11] KVM: SVM: add wrappers to enable/disable IRET interception

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Jan 2023 21:07:46 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>
Cc:     Santosh Shukla <santosh.shukla@....com>, kvm@...r.kernel.org,
        Sandipan Das <sandipan.das@....com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Borislav Petkov <bp@...en8.de>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
        Jiaxi Chen <jiaxi.chen@...ux.intel.com>,
        Babu Moger <babu.moger@....com>, linux-kernel@...r.kernel.org,
        Jing Liu <jing2.liu@...el.com>,
        Wyes Karny <wyes.karny@....com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v2 06/11] KVM: SVM: add wrappers to enable/disable IRET
 interception

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.