Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 11 Mar 2022 02:19:32 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Ben Gardon <bgardon@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>, kvm <kvm@vger.kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Peter Xu <peterx@redhat.com>,
        David Matlack <dmatlack@google.com>,
        Jim Mattson <jmattson@google.com>,
        David Dunn <daviddunn@google.com>,
        Jing Zhang <jingzhangos@google.com>,
        Junaid Shahid <junaids@google.com>
Subject: Re: [PATCH 00/13] KVM: x86: Add a cap to disable NX hugepages on a VM
Message-ID: <YiqxtIz+T1LGE1Ju@google.com>
References: <20220310164532.1821490-1-bgardon@google.com>
 <Yio8QtuMd6COcnEw@google.com>
 <CANgfPd9xr5ev7fEiwBVUi89iHkuywq-Ba9zOeCMSTFmLkO243w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANgfPd9xr5ev7fEiwBVUi89iHkuywq-Ba9zOeCMSTFmLkO243w@mail.gmail.com>
Precedence: bulk

On Thu, Mar 10, 2022, Ben Gardon wrote:
> Those patches are a lot of churn, but at least to me, they make the
> code much more readable. Currently there are many functions which just
> pass along 0 for the memslot, and often have multiple other numerical
> arguments, which makes it hard to understand what the function is
> doing.

Yeah, my solution for that was to rip out all the params.  E.g. the most used
function I ended up with is

  static inline struct kvm_vm *vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
						     void *guest_code)
  {
	return __vm_create_with_one_vcpu(vcpu, 0, guest_code);
  }

and then the usage is

	vm = vm_create_with_one_vcpu(&vcpu, guest_main);

	supp_cpuid = kvm_get_supported_cpuid();
	cpuid2 = vcpu_get_cpuid(vcpu);

My overarching complaint with the selftests is that they make the hard things hard,
and the easy things harder.  If a test wants to be backed by hugepages, it shouldn't
have to manually specify a memslot.

Let me post my selftests rework as RFC (_very_ RFC at this point).  I was hoping to
do more than compile test before posting anything, but it's going to be multiple
weeks before I'll get back to it.  Hopefully it'll start a discussion on actually
rewriting the framework so that writing new tests is less painful, and so that every
new thing that comes along doesn't require poking at 50 different tests.