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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aYU87QeMg8_kTM-G@google.com>
Date: Thu, 5 Feb 2026 16:59:25 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
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

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).