linux-kernel - Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc

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]

Message-ID: <ujghelftsta6zcuyajew74yqtfkhqgdgnsomgyphkfb54itgmd@ouew3jl47nbg>
Date: Wed, 4 Feb 2026 18:24:53 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Kevin Cheng <chengkev@...gle.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in
 recalc_intercepts()

On Wed, Feb 04, 2026 at 09:55:36AM -0800, Sean Christopherson wrote:
> On Wed, Feb 04, 2026, Yosry Ahmed wrote:
> > On Wed, Feb 04, 2026 at 09:29:36AM -0800, Sean Christopherson wrote:
> > > 
> > > >  	for (i = 0; i < MAX_INTERCEPT; i++)
> > > > -		c->intercepts[i] = h->intercepts[i];
> > > > +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
> > > >  
> > > > -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> > > > +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {
> > > 
> > > I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
> > > nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
> > > me, this is waaaay more intuivite:
> > 
> > I agree it reads better, but honestly all of nSVM code uses svm->nested.ctl,
> > and changing its name here just makes things inconsistent imo.
> 
> Gotta start somewhere :-)  In all seriousness, if we didn't allow chipping away
> to at historical oddities in KVM, the code base would be a disaster.  I'm all for
> prioritizing consistency, but I draw the line at "everything else sucks, so this
> needs to suck too".
> 
> I'm not saying we need to do a wholesale rename, but giving at least
> nested_vmcb02_prepare_control() the same treatment will be a huge improvement.
> Actually, I'm going to go do that right now...

For what it's worth, at some point I was going to send a patch to put
svm->nested.ctl and svm->nested.save in an anonymous struct, to end up
with svm->nested.cached_vmcb12.ctl and svm->nested.cached_vmcb12.save,
but the names are too long :)