Re: [PATCH v7 8/9] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
From: Sean Christopherson
Date: Tue Apr 07 2026 - 12:56:19 EST
On Tue, Apr 07, 2026, Jim Mattson wrote:
> On Tue, Apr 7, 2026 at 7:14 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > > use_separate_l2_pat = (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP);
> > > > if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT))
> > > > use_separate_l2_pat = false;
> > >
> > > Wow. I really have no idea how to predict what you're going to want
> > > the code to look like. How is this better than the original?!?
> >
> > It doesn't immediately wrap after the "=". Similar to my view on wrapping before
> > function names[*], I find wrapping immediately after an assignment operator to be
> > unnecessarily difficult to read as it doesn't provide any context for single-line
> > searches.
>
> That's actually a good argument to *never* wrap a line. If a line is
> broken at all, the interesting context might follow the line break.
Don't let perfect be the enemy of good. :-)
> > I'm pretty darn consistent in my dislike for that style: I count 26 instances in
> > arch/x86/kvm that match "\s=\n", and only two of those carry my SoB or R-b. I
> > simply missed the wrap in kvm_vcpu_apicv_activated() that was added by commit
> > 896046474f8d ("KVM: x86: Introduce kvm_x86_call() to simplify static calls of
> > kvm_x86_ops"), and I'll give myself a pass for commit 8764ed55c970 ("KVM: x86:
> > Whitelist port 0x7e for pre-incrementing %rip") as that predates treating
> > checkpatch's 80 char limit as a soft limit.
>
> Might I suggest that you should provide a tool—something like
> checkpatch.pl—that flags style violations?
Or maybe extend checkpatch with an optional "feature"? Or subsystem-specific
rules?