linux-kernel - Re: [PATCH v2 00/23] Extend test coverage for nested SVM

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: <t4modyzuwzmlmu4hcwpxzsbprhebjwuz3uc2doc6nauepruczw@vray2facmzks>
Date: Fri, 21 Nov 2025 00:05:56 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Jim Mattson <jmattson@...gle.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/23] Extend test coverage for nested SVM

On Thu, Nov 20, 2025 at 03:50:10PM -0800, Sean Christopherson wrote:
> On Tue, Oct 21, 2025, Yosry Ahmed wrote:
> > There are multiple selftests exercising nested VMX that are not specific
> > to VMX (at least not anymore). Extend their coverage to nested SVM.
> > 
> > This version is significantly different (and longer) than v1 [1], mainly
> > due to the change of direction to reuse __virt_pg_map() for nested EPT/NPT
> > mappings instead of extending the existing nested EPT infrastructure. It
> > also has a lot more fixups and cleanups.
> > 
> > This series depends on two other series:
> > - "KVM: SVM: GIF and EFER.SVME are independent" [2]
> > - "KVM: selftests: Add test of SET_NESTED_STATE with 48-bit L2 on 57-bit L1" [3]
> > 
> > The dependency on the former is because set_nested_state_test is now
> > also a regression test for that fix.
> 
> Uh, then the selftest change absolutely should be sent at the same time as the
> KVM change.  One of the big benefits of selftests over KUT is that selftests are
> in the same repo as KVM.  We should almost never have to coordinate selftests
> chagnes against KVM changes across different series.

Yeah that didn't work out well. I saw Jim's fixes as I was working on
that test and thought might as well test for Jim's changes. In
retrospect I should have split this into two patches.

> 
> > The dependency on the latter is purely to avoid conflicts.
> 
> Similar to my feedback on your mega-series for KUT, don't bundle unrelated patches
> without good reason (and no reason _NOT_ to bundle them).

Noted.

> 
> I want to immediate take the patches that aren't related to the paging API changes,
> but that's proving to be difficult because there are superficial dependencies on
> Jim's LA57 changes, and I need to drop the vmx_set_nested_state_test changes because
> they belong elsewhere.
> 
> Bundling these is fine since they're thematically related and do generate superficial
> conflicts, though even then I would be a-ok with splitting these up (superficial
> conflicts are trivial to resolve (knock wood), and avoiding such conflicts isn't
> a good reason to bundle unrelated things).
> 
>   KVM: selftests: Extend vmx_tsc_adjust_test to cover SVM
>   KVM: selftests: Extend nested_invalid_cr3_test to cover SVM
>   KVM: selftests: Move nested invalid CR3 check to its own test
>   KVM: selftests: Extend vmx_nested_tsc_scaling_test to cover SVM
>   KVM: selftests: Extend vmx_close_while_nested_test to cover SVM

Not sure I understand how you to proceed. Do you want me to respin these
patches separately (as series A), on top of kvm-x86/next, and then
respin the rest of the series separately (as series B, with your struct
kvm_mmu suggestion)?

As for set_nested_state, if you plan to pickup Jim's EFER fixes I can
just include it as-is in series (A). If not, I can include
generalization of the test, and send covering Jim's fix separately.

Series B will still need to depend on Jim's selftests changes, if you're
planning to pick those up soon I can base my changes on whatever branch
you'll use. Otherwise I can resend both together, maybe?