Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220310164532.1821490-1-bgardon@google.com> <Yio8QtuMd6COcnEw@google.com>
 <CANgfPd9xr5ev7fEiwBVUi89iHkuywq-Ba9zOeCMSTFmLkO243w@mail.gmail.com> <YiqxtIz+T1LGE1Ju@google.com>
In-Reply-To: <YiqxtIz+T1LGE1Ju@google.com>
From:   Ben Gardon <bgardon@google.com>
Date:   Tue, 15 Mar 2022 17:12:29 -0600
Message-ID: <CANgfPd9N0UsA7_uMU4vs43gdH7A3UrcrY-OdmXoxQ2PyctwxsA@mail.gmail.com>
Subject: Re: [PATCH 00/13] KVM: x86: Add a cap to disable NX hugepages on a VM
To:     Sean Christopherson <seanjc@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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Okay, I'll hold off on the memslot refactor in v2, but if folks have
feedback on the other aspects of the v1 patch series, I'd appreciate
it.
If not, I'll try to get a v2 sent out.
I think that the commits adding utility functions for the binary stats
interface to the binary stats test could be queued separately from the
rest of this series and will be helpful for other folks working on new
selftests.

On Thu, Mar 10, 2022 at 8:19 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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.