linux-kernel - Re: [PATCH v4 01/26] KVM: SVM: Switch svm_copy

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b92c2a7c7bcdc02d49eb0c0d481f682bf5d10c76@linux.dev>
Date: Fri, 06 Feb 2026 01:12:38 +0000
From: "Yosry Ahmed" <yosry.ahmed@...ux.dev>
To: "Sean Christopherson" <seanjc@...gle.com>
Cc: "Paolo Bonzini" <pbonzini@...hat.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v4 01/26] KVM: SVM: Switch svm_copy_lbrs() to a macro

February 5, 2026 at 4:59 PM, "Sean Christopherson" <seanjc@...gle.com> wrote:


> 
> On Thu, Jan 15, 2026, Yosry Ahmed wrote:
> 
> > 
> > In preparation for using svm_copy_lbrs() with 'struct vmcb_save_area'
> >  without a containing 'struct vmcb', and later even 'struct
> >  vmcb_save_area_cached', make it a macro. Pull the call to
> >  vmcb_mark_dirty() out to the callers.
> >  
> >  Macros are generally not preferred compared to functions, mainly due to
> >  type-safety. However, in this case it seems like having a simple macro
> >  copying a few fields is better than copy-pasting the same 5 lines of
> >  code in different places.
> >  
> >  On the bright side, pulling vmcb_mark_dirty() calls to the callers makes
> >  it clear that in one case, vmcb_mark_dirty() was being called on VMCB12.
> >  It is not architecturally defined for the CPU to clear arbitrary clean
> >  bits, and it is not needed, so drop that one call.
> >  
> >  Technically fixes the non-architectural behavior of setting the dirty
> >  bit on VMCB12.
> > 
> Stop. Bundling. Things. Together.
> 
> /shakes fist angrily
> 
> I was absolutely not expecting a patch titled "KVM: SVM: Switch svm_copy_lbrs()
> to a macro" to end with a Fixes tag, and I was *really* not expecting it to also
> be Cc'd for stable.
> 
> At a glance, I genuinely can't tell if you added a Fixes to scope the backport,
> or because of the dirty vmcb12 bits thing.
> 
> First fix the dirty behavior (and probably tag it for stable to avoid creating
> an unnecessary backport conflict), then in a separate patch macrofy the helper.
> Yeah, checkpatch will "suggest" that the stable@ patch should have Fixes, but
> for us humans, that's _useful_ information, because it says "hey you, this is a
> dependency for an upcoming fix!". As written, I look at this patch and go "huh?".
> (and then I look at the next patch and it all makes sense).

I agree, but fixing the dirty behavior on its own requires open-coding the function, then the following patch would change it to a macro and use it again. I was trying to minimize the noise of moving code back and forth..