Create mmu_stress_tests's VM with the correct number of extra pages needed
to map all of memory in the guest.  The bug hasn't been noticed before as
the test currently runs only on x86, which maps guest memory with 1GiB
pages, i.e. doesn't need much memory in the guest for page tables.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/mmu_stress_test.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 847da23ec1b1..5467b12f5903 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -209,7 +209,13 @@ int main(int argc, char *argv[])
 	vcpus = malloc(nr_vcpus * sizeof(*vcpus));
 	TEST_ASSERT(vcpus, "Failed to allocate vCPU array");
 
-	vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
+	vm = __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus,
+#ifdef __x86_64__
+				    max_mem / SZ_1G,
+#else
+				    max_mem / vm_guest_mode_params[VM_MODE_DEFAULT].page_size,
+#endif
+				    guest_code, vcpus);
 
 	max_gpa = vm->max_gfn << vm->page_shift;
 	TEST_ASSERT(max_gpa > (4 * slot_size), "MAXPHYADDR <4gb ");
-- 
2.46.0.76.ge559c4bf1a-goog

On Fri, Aug 9, 2024 at 12:43 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Create mmu_stress_tests's VM with the correct number of extra pages needed
> to map all of memory in the guest.  The bug hasn't been noticed before as
> the test currently runs only on x86, which maps guest memory with 1GiB
> pages, i.e. doesn't need much memory in the guest for page tables.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/mmu_stress_test.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
> index 847da23ec1b1..5467b12f5903 100644
> --- a/tools/testing/selftests/kvm/mmu_stress_test.c
> +++ b/tools/testing/selftests/kvm/mmu_stress_test.c
> @@ -209,7 +209,13 @@ int main(int argc, char *argv[])
>         vcpus = malloc(nr_vcpus * sizeof(*vcpus));
>         TEST_ASSERT(vcpus, "Failed to allocate vCPU array");
>
> -       vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
> +       vm = __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus,
> +#ifdef __x86_64__
> +                                   max_mem / SZ_1G,
> +#else
> +                                   max_mem / vm_guest_mode_params[VM_MODE_DEFAULT].page_size,
> +#endif
> +                                   guest_code, vcpus);

Hmm... I'm trying to square this change with the logic in
vm_nr_pages_required(). That logic seems to be doing what you want
(though it always assumes small mappings IIUC).

So it seems like there's something else that's not being accounted
for? (Also without the extra pages, how does this test actually fail?)

On Thu, Sep 05, 2024, James Houghton wrote:
> On Fri, Aug 9, 2024 at 12:43 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Create mmu_stress_tests's VM with the correct number of extra pages needed
> > to map all of memory in the guest.  The bug hasn't been noticed before as
> > the test currently runs only on x86, which maps guest memory with 1GiB
> > pages, i.e. doesn't need much memory in the guest for page tables.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  tools/testing/selftests/kvm/mmu_stress_test.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
> > index 847da23ec1b1..5467b12f5903 100644
> > --- a/tools/testing/selftests/kvm/mmu_stress_test.c
> > +++ b/tools/testing/selftests/kvm/mmu_stress_test.c
> > @@ -209,7 +209,13 @@ int main(int argc, char *argv[])
> >         vcpus = malloc(nr_vcpus * sizeof(*vcpus));
> >         TEST_ASSERT(vcpus, "Failed to allocate vCPU array");
> >
> > -       vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
> > +       vm = __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus,
> > +#ifdef __x86_64__
> > +                                   max_mem / SZ_1G,
> > +#else
> > +                                   max_mem / vm_guest_mode_params[VM_MODE_DEFAULT].page_size,
> > +#endif
> > +                                   guest_code, vcpus);
> 
> Hmm... I'm trying to square this change with the logic in
> vm_nr_pages_required(). 

vm_nr_pages_required() mostly operates on the number of pages that are needed to
setup the VM, e.g. for vCPU stacks.  The one calculation that guesstimates the
number of bytes needed, ucall_nr_pages_required(), does the same thing this code
does: divide the number of total bytes by bytes-per-page.

> That logic seems to be doing what you want (though it always assumes small
> mappings IIUC).

Ya, AFAIK, x86 is the only architecture that let's test map huge pages on the
guest side.

> So it seems like there's something else that's not being accounted for?

I don't think so?  vm_nr_pages_required() uses @nr_pages to determine how many
page table pages will be needed in the guest, and then adds that many non-huge
pages worth of bytes to the size of memslot 0.

> (Also without the extra pages, how does this test actually fail?)

Guest memory allocation failure when trying to create the guest mappings.

On Thu, Sep 5, 2024 at 9:42 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Sep 05, 2024, James Houghton wrote:
> > On Fri, Aug 9, 2024 at 12:43 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Create mmu_stress_tests's VM with the correct number of extra pages needed
> > > to map all of memory in the guest.  The bug hasn't been noticed before as
> > > the test currently runs only on x86, which maps guest memory with 1GiB
> > > pages, i.e. doesn't need much memory in the guest for page tables.
> > >
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >  tools/testing/selftests/kvm/mmu_stress_test.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
> > > index 847da23ec1b1..5467b12f5903 100644
> > > --- a/tools/testing/selftests/kvm/mmu_stress_test.c
> > > +++ b/tools/testing/selftests/kvm/mmu_stress_test.c
> > > @@ -209,7 +209,13 @@ int main(int argc, char *argv[])
> > >         vcpus = malloc(nr_vcpus * sizeof(*vcpus));
> > >         TEST_ASSERT(vcpus, "Failed to allocate vCPU array");
> > >
> > > -       vm = vm_create_with_vcpus(nr_vcpus, guest_code, vcpus);
> > > +       vm = __vm_create_with_vcpus(VM_SHAPE_DEFAULT, nr_vcpus,
> > > +#ifdef __x86_64__
> > > +                                   max_mem / SZ_1G,
> > > +#else
> > > +                                   max_mem / vm_guest_mode_params[VM_MODE_DEFAULT].page_size,
> > > +#endif
> > > +                                   guest_code, vcpus);
> >
> > Hmm... I'm trying to square this change with the logic in
> > vm_nr_pages_required().
>
> vm_nr_pages_required() mostly operates on the number of pages that are needed to
> setup the VM, e.g. for vCPU stacks.  The one calculation that guesstimates the
> number of bytes needed, ucall_nr_pages_required(), does the same thing this code
> does: divide the number of total bytes by bytes-per-page.

Oh, yes, you're right. It's only accounting for the page tables for
the 512 pages for memslot 0. Sorry for the noise. Feel free to add:

Reviewed-by: James Houghton <jthoughton@google.com>