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;
MIME-Version: 1.0
References: <20220310164532.1821490-1-bgardon@google.com> <Yio8QtuMd6COcnEw@google.com>
In-Reply-To: <Yio8QtuMd6COcnEw@google.com>
From:   Ben Gardon <bgardon@google.com>
Date:   Thu, 10 Mar 2022 13:29:15 -0600
Message-ID: <CANgfPd9xr5ev7fEiwBVUi89iHkuywq-Ba9zOeCMSTFmLkO243w@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

On Thu, Mar 10, 2022 at 11:58 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Mar 10, 2022, Ben Gardon wrote:
> >   selftests: KVM: Wrap memslot IDs in a struct for readability
> >   selftests: KVM: Add memslot parameter to VM vaddr allocation
> >   selftests: KVM: Add memslot parameter to elf_load
>
> I really, really, don't want to go down this path of proliferating memslot crud
> throughout the virtual memory allocators.  I would much rather we solve this by
> teaching the VM creation helpers to (optionally) use hugepages.  The amount of
> churn required just so that one test can back code with hugepages is absurd, and
> there's bound to be tests in the future that want to force hugepages as well.

I agree that proliferating the memslots argument isn't strictly
required for this test, but doing so makes it much easier to make
assertions about hugepage counts and such because you don't have your
stacks and page tables backed with hugepages.

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.

I don't think explicitly specifying memslots really adds that much
overhead to the tests, and I'd rather have control over that than
implicitly cramming everything into memslot 0.

If you have a better way to manage the memslots and create virtual
mappings for / load code into other memslots, I'm open to it, but we
should do something about it.