From: Vishal Annapurve <vannapurve@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: x86@kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com,
jmattson@google.com, joro@8bytes.org, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, shuah@kernel.org, yang.zhong@intel.com,
ricarkol@google.com, aaronlewis@google.com, wei.w.wang@intel.com,
kirill.shutemov@linux.intel.com, corbet@lwn.net,
hughd@google.com, jlayton@kernel.org, bfields@fieldses.org,
akpm@linux-foundation.org, chao.p.peng@linux.intel.com,
yu.c.zhang@linux.intel.com, jun.nakajima@intel.com,
dave.hansen@intel.com, michael.roth@amd.com, qperret@google.com,
steven.price@arm.com, ak@linux.intel.com, david@redhat.com,
luto@kernel.org, vbabka@suse.cz, marcorr@google.com,
erdemaktas@google.com, pgonda@google.com, nikunj@amd.com,
diviness@google.com, maz@kernel.org, dmatlack@google.com,
axelrasmussen@google.com, maciej.szmigiero@oracle.com,
mizhang@google.com, bgardon@google.com, ackerleytng@google.com
Subject: Re: [V2 PATCH 2/6] KVM: Selftests: Add support for private memory
Date: Tue, 17 Jan 2023 15:03:06 -0800 [thread overview]
Message-ID: <CAGtprH-jXuYs4MqNupxi2aEQ3ohp0PyqpNZd0UDu8yqF+57aXw@mail.gmail.com> (raw)
In-Reply-To: <Y8cXMCBzNcuzniXS@google.com>
On Tue, Jan 17, 2023 at 1:46 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Dec 05, 2022, Vishal Annapurve wrote:
> > Add support for registering private memory with kvm using
> > KVM_SET_USER_MEMORY_REGION ioctl.
> >
> > Helper function to query extended userspace mem region is introduced to
> > allow memory conversion.
> >
> > vm_mem_backing_src types is extended to contain additional guest memory
> > source types to cover the cases where guest memory can be backed by both
> > anonymous memory and restricted memfd.
> >
> > Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> > ---
> > .../selftests/kvm/include/kvm_util_base.h | 12 +++-
> > .../testing/selftests/kvm/include/test_util.h | 4 ++
> > tools/testing/selftests/kvm/lib/kvm_util.c | 58 +++++++++++++++++--
> > tools/testing/selftests/kvm/lib/test_util.c | 11 ++++
> > 4 files changed, 78 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> > index c7685c7038ff..4ad99f295f2a 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> > @@ -31,7 +31,10 @@ typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */
> > typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */
> >
> > struct userspace_mem_region {
> > - struct kvm_userspace_memory_region region;
> > + union {
> > + struct kvm_userspace_memory_region region;
> > + struct kvm_userspace_memory_region_ext region_ext;
>
> As discussed in the UPM series, we're trending towards adding an entirely new
> struct+ioctl(), kvm_userspace_memory_region2, instead of extending the existing
> struct. The == -> >= hack you had to add in kvm_do_ioctl() below is one of the
> reason for that change.
>
Ack.
> > + };
> > struct sparsebit *unused_phy_pages;
> > int fd;
> > off_t offset;
> > @@ -196,7 +199,7 @@ static inline bool kvm_has_cap(long cap)
> >
> > #define kvm_do_ioctl(fd, cmd, arg) \
> > ({ \
> > - static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd), ""); \
> > + static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) >= _IOC_SIZE(cmd), ""); \
> > ioctl(fd, cmd, arg); \
> > })
> >
> > @@ -384,6 +387,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> > void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
> > void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
> > void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
> > +
> > struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id);
> > vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
> > vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
> > @@ -715,6 +719,10 @@ struct kvm_userspace_memory_region *
> > kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start,
> > uint64_t end);
> >
> > +struct kvm_userspace_memory_region_ext *
> > +kvm_userspace_memory_region_ext_find(struct kvm_vm *vm, uint64_t start,
> > + uint64_t end);
> > +
> > #define sync_global_to_guest(vm, g) ({ \
> > typeof(g) *_p = addr_gva2hva(vm, (vm_vaddr_t)&(g)); \
> > memcpy(_p, &(g), sizeof(g)); \
> > diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> > index 80d6416f3012..aea80071f2b8 100644
> > --- a/tools/testing/selftests/kvm/include/test_util.h
> > +++ b/tools/testing/selftests/kvm/include/test_util.h
> > @@ -103,6 +103,8 @@ enum vm_mem_backing_src_type {
> > VM_MEM_SRC_ANONYMOUS_HUGETLB_16GB,
> > VM_MEM_SRC_SHMEM,
> > VM_MEM_SRC_SHARED_HUGETLB,
> > + VM_MEM_SRC_ANONYMOUS_AND_RESTRICTED_MEMFD,
> > + VM_MEM_SRC_ANON_HTLB2M_AND_RESTRICTED_MEMFD,
>
> There's no need for a dedicated flag in the backing type, vm_userspace_mem_region_add()
> already takes the memslot's flags and can simply key off KVM_MEM_PRIVATE.
>
I switched to using a dedicated flag thinking that it might be handy
when private memory can be backed by huge pages. For now it makes
sense to avoid adding it.
> > @@ -881,6 +915,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> > struct userspace_mem_region *region;
> > size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
> > size_t alignment;
> > + int restricted_memfd = -1;
>
> No need to initialize to -1, KVM is supposed to ignore the restrictedmem fd if
> !KVM_MEM_PRIVATE, and if KVM_MEM_PRIVATE is set, selftests must provide a valid fd.
>
> > TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
> > "Number of guest pages is not compatible with the host. "
>
> This is what I ended up with after splitting out the conversion to
> KVM_SET_USER_MEMORY_REGION2 to a separate patch.
>
> --
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 7c1f81f93ba3..26c6830c1aa1 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -32,6 +32,11 @@ int open_path_or_exit(const char *path, int flags)
> return fd;
> }
>
> +static int memfd_restricted(unsigned int flags)
> +{
> + return syscall(__NR_memfd_restricted, flags);
> +}
> +
> /*
> * Open KVM_DEV_PATH if available, otherwise exit the entire program.
> *
> @@ -980,6 +985,15 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> }
>
> region->backing_src_type = src_type;
> +
> + if (flags & KVM_MEM_PRIVATE) {
> + region->region.restricted_fd = memfd_restricted(0);
> + region->region.restricted_offset = 0;
> +
> + TEST_ASSERT(region->region.restricted_fd >= 0,
> + "Failed to create restricted memfd");
> + }
> +
> region->unused_phy_pages = sparsebit_alloc();
> sparsebit_set_num(region->unused_phy_pages,
> guest_paddr >> vm->page_shift, npages);
> @@ -992,9 +1006,10 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION2 IOCTL failed,\n"
> " rc: %i errno: %i\n"
> " slot: %u flags: 0x%x\n"
> - " guest_phys_addr: 0x%lx size: 0x%lx",
> + " guest_phys_addr: 0x%lx size: 0x%lx restricted fd: %d\n",
> ret, errno, slot, flags,
> - guest_paddr, (uint64_t) region->region.memory_size);
> + guest_paddr, (uint64_t) region->region.memory_size,
> + region->region.restricted_fd);
>
> /* Add to quick lookup data structures */
> vm_userspace_mem_region_gpa_insert(&vm->regions.gpa_tree, region);
Ack.
next prev parent reply other threads:[~2023-01-17 23:52 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-05 23:23 [V2 PATCH 0/6] KVM: selftests: selftests for fd-based private memory Vishal Annapurve
2022-12-05 23:23 ` [V2 PATCH 1/6] KVM: x86: Add support for testing " Vishal Annapurve
2023-01-17 21:39 ` Sean Christopherson
2023-01-17 22:58 ` Vishal Annapurve
2022-12-05 23:23 ` [V2 PATCH 2/6] KVM: Selftests: Add support for " Vishal Annapurve
2023-01-17 21:46 ` Sean Christopherson
2023-01-17 23:03 ` Vishal Annapurve [this message]
2022-12-05 23:23 ` [V2 PATCH 3/6] KVM: selftests: x86: Add IS_ALIGNED/IS_PAGE_ALIGNED helpers Vishal Annapurve
2023-01-17 21:48 ` Sean Christopherson
2023-01-17 23:06 ` Vishal Annapurve
2022-12-05 23:23 ` [V2 PATCH 4/6] KVM: selftests: x86: Add helpers to execute VMs with private memory Vishal Annapurve
2023-01-17 22:06 ` Sean Christopherson
2023-01-17 22:51 ` Sean Christopherson
2022-12-05 23:23 ` [V2 PATCH 5/6] KVM: selftests: Add get_free_huge_2m_pages Vishal Annapurve
2023-01-17 22:12 ` Sean Christopherson
2022-12-05 23:23 ` [V2 PATCH 6/6] KVM: selftests: x86: Add selftest for private memory Vishal Annapurve
2023-01-18 0:53 ` Sean Christopherson
2023-01-18 1:09 ` [V2 PATCH 0/6] KVM: selftests: selftests for fd-based " Sean Christopherson
2023-01-18 3:11 ` Vishal Annapurve
2023-02-10 19:59 ` Vishal Annapurve
2023-02-22 2:50 ` Chao Peng
2023-03-06 18:21 ` Ackerley Tng
2023-03-07 2:18 ` Chao Peng
2023-03-08 1:59 ` Ackerley Tng
2023-03-08 20:11 ` Sean Christopherson
2023-01-18 11:25 ` Kirill A. Shutemov
2023-01-18 17:17 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGtprH-jXuYs4MqNupxi2aEQ3ohp0PyqpNZd0UDu8yqF+57aXw@mail.gmail.com \
--to=vannapurve@google.com \
--cc=aaronlewis@google.com \
--cc=ackerleytng@google.com \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=axelrasmussen@google.com \
--cc=bfields@fieldses.org \
--cc=bgardon@google.com \
--cc=bp@alien8.de \
--cc=chao.p.peng@linux.intel.com \
--cc=corbet@lwn.net \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=david@redhat.com \
--cc=diviness@google.com \
--cc=dmatlack@google.com \
--cc=erdemaktas@google.com \
--cc=hpa@zytor.com \
--cc=hughd@google.com \
--cc=jlayton@kernel.org \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=jun.nakajima@intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=luto@kernel.org \
--cc=maciej.szmigiero@oracle.com \
--cc=marcorr@google.com \
--cc=maz@kernel.org \
--cc=michael.roth@amd.com \
--cc=mingo@redhat.com \
--cc=mizhang@google.com \
--cc=nikunj@amd.com \
--cc=pbonzini@redhat.com \
--cc=pgonda@google.com \
--cc=qperret@google.com \
--cc=ricarkol@google.com \
--cc=seanjc@google.com \
--cc=shuah@kernel.org \
--cc=steven.price@arm.com \
--cc=tglx@linutronix.de \
--cc=vbabka@suse.cz \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=wei.w.wang@intel.com \
--cc=x86@kernel.org \
--cc=yang.zhong@intel.com \
--cc=yu.c.zhang@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).