linux-kernel - Re: [PATCH v3 8/8] KVM: selftests: Move large memslots above KVM internal memslots in _vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Jan 2020 13:20:53 -0800
From:   Ben Gardon <bgardon@...gle.com>
To:     Andrew Jones <drjones@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Cannon Matthews <cannonmatthews@...gle.com>,
        Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH v3 8/8] KVM: selftests: Move large memslots above KVM
 internal memslots in _vm_create

Would it be viable to allocate at 4G be default and then add another
interface for allocations at low memory addresses? For most tests, I
don't think there's any value to having the backing paddrs below 3G.

On Tue, Jan 7, 2020 at 7:42 AM Andrew Jones <drjones@...hat.com> wrote:
>
> On Mon, Dec 16, 2019 at 01:39:01PM -0800, Ben Gardon wrote:
> > KVM creates internal memslots between 3 and 4 GiB paddrs on the first
> > vCPU creation. If memslot 0 is large enough it collides with these
> > memslots an causes vCPU creation to fail. When requesting more than 3G,
> > start memslot 0 at 4G in _vm_create.
> >
> > Signed-off-by: Ben Gardon <bgardon@...gle.com>
> > ---
> >  tools/testing/selftests/kvm/lib/kvm_util.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 41cf45416060f..886d58e6cac39 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -113,6 +113,8 @@ const char * const vm_guest_mode_string[] = {
> >  _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
> >              "Missing new mode strings?");
> >
> > +#define KVM_INTERNAL_MEMSLOTS_START_PADDR (3UL << 30)
> > +#define KVM_INTERNAL_MEMSLOTS_END_PADDR (4UL << 30)
> >  /*
> >   * VM Create
> >   *
> > @@ -128,13 +130,16 @@ _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
> >   *
> >   * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> >   * When phy_pages is non-zero, a memory region of phy_pages physical pages
> > - * is created and mapped starting at guest physical address 0.  The file
> > - * descriptor to control the created VM is created with the permissions
> > - * given by perm (e.g. O_RDWR).
> > + * is created. If phy_pages is less that 3G, it is mapped starting at guest
> > + * physical address 0. If phy_pages is greater than 3G it is mapped starting
> > + * 4G into the guest physical address space to avoid KVM internal memslots
> > + * which map the region between 3G and 4G. The file descriptor to control the
> > + * created VM is created with the permissions given by perm (e.g. O_RDWR).
> >   */
> >  struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> >  {
> >       struct kvm_vm *vm;
> > +     uint64_t guest_paddr = 0;
> >
> >       DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode));
> >
> > @@ -227,9 +232,11 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> >
> >       /* Allocate and setup memory for guest. */
> >       vm->vpages_mapped = sparsebit_alloc();
> > +     if (guest_paddr + phy_pages > KVM_INTERNAL_MEMSLOTS_START_PADDR)
> > +             guest_paddr = KVM_INTERNAL_MEMSLOTS_END_PADDR;
> >       if (phy_pages != 0)
> >               vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> > -                                         0, 0, phy_pages, 0);
> > +                                         guest_paddr, 0, phy_pages, 0);
> >
> >       return vm;
> >  }
> > --
> > 2.24.1.735.g03f4e72817-goog
> >
>
> I feel like this function is becoming too magic and it'll be more
> complicated for tests that need to add additional memory regions
> to know what physical addresses are available. Maybe we should assert
> if we can't allocate more than 3G at offset zero and also provide
> another interface for allocating at an offset input by the user,
> as long as the offset is 4G or above (asserting when it isn't)?
>
> Thanks,
> drew
>