Add and use wrappers for mmap() and munmap() that assert success to reduce
a significant amount of boilerplate code, to ensure all tests assert on
failure, and to provide consistent error messages on failure.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
.../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
.../selftests/kvm/s390/ucontrol_test.c | 16 +++----
.../selftests/kvm/set_memory_region_test.c | 17 ++++---
6 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
index 5a50a28ce1fa..5dd40b77dc07 100644
--- a/tools/testing/selftests/kvm/guest_memfd_test.c
+++ b/tools/testing/selftests/kvm/guest_memfd_test.c
@@ -50,8 +50,7 @@ static void test_mmap_supported(int fd, size_t total_size)
mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
- mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- TEST_ASSERT(mem != MAP_FAILED, "mmap() for guest_memfd should succeed.");
+ mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
memset(mem, val, total_size);
for (i = 0; i < total_size; i++)
@@ -70,8 +69,7 @@ static void test_mmap_supported(int fd, size_t total_size)
for (i = 0; i < total_size; i++)
TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
- ret = munmap(mem, total_size);
- TEST_ASSERT(!ret, "munmap() should succeed.");
+ kvm_munmap(mem, total_size);
}
static sigjmp_buf jmpbuf;
@@ -89,10 +87,8 @@ static void test_fault_overflow(int fd, size_t total_size)
const char val = 0xaa;
char *mem;
size_t i;
- int ret;
- mem = mmap(NULL, map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- TEST_ASSERT(mem != MAP_FAILED, "mmap() for guest_memfd should succeed.");
+ mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
sigaction(SIGBUS, &sa_new, &sa_old);
if (sigsetjmp(jmpbuf, 1) == 0) {
@@ -104,8 +100,7 @@ static void test_fault_overflow(int fd, size_t total_size)
for (i = 0; i < total_size; i++)
TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
- ret = munmap(mem, map_size);
- TEST_ASSERT(!ret, "munmap() should succeed.");
+ kvm_munmap(mem, map_size);
}
static void test_mmap_not_supported(int fd, size_t total_size)
@@ -347,10 +342,9 @@ static void test_guest_memfd_guest(void)
GUEST_MEMFD_FLAG_DEFAULT_SHARED);
vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
- mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- TEST_ASSERT(mem != MAP_FAILED, "mmap() on guest_memfd failed");
+ mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
memset(mem, 0xaa, size);
- munmap(mem, size);
+ kvm_munmap(mem, size);
virt_pg_map(vm, gpa, gpa);
vcpu_args_set(vcpu, 2, gpa, size);
@@ -358,8 +352,7 @@ static void test_guest_memfd_guest(void)
TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
- mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- TEST_ASSERT(mem != MAP_FAILED, "mmap() on guest_memfd failed");
+ mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
for (i = 0; i < size; i++)
TEST_ASSERT_EQ(mem[i], 0xff);
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 23a506d7eca3..1c68ff0fb3fb 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
#define __KVM_SYSCALL_ERROR(_name, _ret) \
"%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
+static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
+ off_t offset)
+{
+ void *mem;
+
+ mem = mmap(NULL, size, prot, flags, fd, offset);
+ TEST_ASSERT(mem != MAP_FAILED, __KVM_SYSCALL_ERROR("mmap()",
+ (int)(unsigned long)MAP_FAILED));
+
+ return mem;
+}
+
+static inline void *kvm_mmap(size_t size, int prot, int flags, int fd)
+{
+ return __kvm_mmap(size, prot, flags, fd, 0);
+}
+
+static inline void kvm_munmap(void *mem, size_t size)
+{
+ int ret;
+
+ ret = munmap(mem, size);
+ TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
+}
+
/*
* Use the "inner", double-underscore macro when reporting errors from within
* other macros so that the name of ioctl() and not its literal numeric value
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index c3f5142b0a54..da754b152c11 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -770,13 +770,11 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
int ret;
if (vcpu->dirty_gfns) {
- ret = munmap(vcpu->dirty_gfns, vm->dirty_ring_size);
- TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
+ kvm_munmap(vcpu->dirty_gfns, vm->dirty_ring_size);
vcpu->dirty_gfns = NULL;
}
- ret = munmap(vcpu->run, vcpu_mmap_sz());
- TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
+ kvm_munmap(vcpu->run, vcpu_mmap_sz());
ret = close(vcpu->fd);
TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
@@ -810,20 +808,16 @@ void kvm_vm_release(struct kvm_vm *vmp)
static void __vm_mem_region_delete(struct kvm_vm *vm,
struct userspace_mem_region *region)
{
- int ret;
-
rb_erase(®ion->gpa_node, &vm->regions.gpa_tree);
rb_erase(®ion->hva_node, &vm->regions.hva_tree);
hash_del(®ion->slot_node);
sparsebit_free(®ion->unused_phy_pages);
sparsebit_free(®ion->protected_phy_pages);
- ret = munmap(region->mmap_start, region->mmap_size);
- TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
+ kvm_munmap(region->mmap_start, region->mmap_size);
if (region->fd >= 0) {
/* There's an extra map when using shared memory. */
- ret = munmap(region->mmap_alias, region->mmap_size);
- TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
+ kvm_munmap(region->mmap_alias, region->mmap_size);
close(region->fd);
}
if (region->region.guest_memfd >= 0)
@@ -1080,12 +1074,9 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
region->fd = kvm_memfd_alloc(region->mmap_size,
src_type == VM_MEM_SRC_SHARED_HUGETLB);
- region->mmap_start = mmap(NULL, region->mmap_size,
- PROT_READ | PROT_WRITE,
- vm_mem_backing_src_alias(src_type)->flag,
- region->fd, 0);
- TEST_ASSERT(region->mmap_start != MAP_FAILED,
- __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
+ region->mmap_start = kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE,
+ vm_mem_backing_src_alias(src_type)->flag,
+ region->fd);
TEST_ASSERT(!is_backing_src_hugetlb(src_type) ||
region->mmap_start == align_ptr_up(region->mmap_start, backing_src_pagesz),
@@ -1156,12 +1147,10 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
/* If shared memory, create an alias. */
if (region->fd >= 0) {
- region->mmap_alias = mmap(NULL, region->mmap_size,
- PROT_READ | PROT_WRITE,
- vm_mem_backing_src_alias(src_type)->flag,
- region->fd, 0);
- TEST_ASSERT(region->mmap_alias != MAP_FAILED,
- __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
+ region->mmap_alias = kvm_mmap(region->mmap_size,
+ PROT_READ | PROT_WRITE,
+ vm_mem_backing_src_alias(src_type)->flag,
+ region->fd);
/* Align host alias address */
region->host_alias = align_ptr_up(region->mmap_alias, alignment);
@@ -1371,10 +1360,8 @@ struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->run), "vcpu mmap size "
"smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
vcpu_mmap_sz(), sizeof(*vcpu->run));
- vcpu->run = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
- PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
- TEST_ASSERT(vcpu->run != MAP_FAILED,
- __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
+ vcpu->run = kvm_mmap(vcpu_mmap_sz(), PROT_READ | PROT_WRITE,
+ MAP_SHARED, vcpu->fd);
if (kvm_has_cap(KVM_CAP_BINARY_STATS_FD))
vcpu->stats.fd = vcpu_get_stats_fd(vcpu);
@@ -1821,9 +1808,8 @@ void *vcpu_map_dirty_ring(struct kvm_vcpu *vcpu)
page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
TEST_ASSERT(addr == MAP_FAILED, "Dirty ring mapped exec");
- addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd,
- page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
- TEST_ASSERT(addr != MAP_FAILED, "Dirty ring map failed");
+ addr = __kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd,
+ page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
vcpu->dirty_gfns = addr;
vcpu->dirty_gfns_count = size / sizeof(struct kvm_dirty_gfn);
diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
index 6a437d2be9fa..37b7e6524533 100644
--- a/tools/testing/selftests/kvm/mmu_stress_test.c
+++ b/tools/testing/selftests/kvm/mmu_stress_test.c
@@ -339,8 +339,7 @@ int main(int argc, char *argv[])
TEST_ASSERT(max_gpa > (4 * slot_size), "MAXPHYADDR <4gb ");
fd = kvm_memfd_alloc(slot_size, hugepages);
- mem = mmap(NULL, slot_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- TEST_ASSERT(mem != MAP_FAILED, "mmap() failed");
+ mem = kvm_mmap(slot_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
TEST_ASSERT(!madvise(mem, slot_size, MADV_NOHUGEPAGE), "madvise() failed");
@@ -413,7 +412,7 @@ int main(int argc, char *argv[])
for (slot = (slot - 1) & ~1ull; slot >= first_slot; slot -= 2)
vm_set_user_memory_region(vm, slot, 0, 0, 0, NULL);
- munmap(mem, slot_size / 2);
+ kvm_munmap(mem, slot_size / 2);
/* Sanity check that the vCPUs actually ran. */
for (i = 0; i < nr_vcpus; i++)
diff --git a/tools/testing/selftests/kvm/s390/ucontrol_test.c b/tools/testing/selftests/kvm/s390/ucontrol_test.c
index d265b34c54be..50bc1c38225a 100644
--- a/tools/testing/selftests/kvm/s390/ucontrol_test.c
+++ b/tools/testing/selftests/kvm/s390/ucontrol_test.c
@@ -142,19 +142,17 @@ FIXTURE_SETUP(uc_kvm)
self->kvm_run_size = ioctl(self->kvm_fd, KVM_GET_VCPU_MMAP_SIZE, NULL);
ASSERT_GE(self->kvm_run_size, sizeof(struct kvm_run))
TH_LOG(KVM_IOCTL_ERROR(KVM_GET_VCPU_MMAP_SIZE, self->kvm_run_size));
- self->run = (struct kvm_run *)mmap(NULL, self->kvm_run_size,
- PROT_READ | PROT_WRITE, MAP_SHARED, self->vcpu_fd, 0);
- ASSERT_NE(self->run, MAP_FAILED);
+ self->run = kvm_mmap(self->kvm_run_size, PROT_READ | PROT_WRITE,
+ MAP_SHARED, self->vcpu_fd);
/**
* For virtual cpus that have been created with S390 user controlled
* virtual machines, the resulting vcpu fd can be memory mapped at page
* offset KVM_S390_SIE_PAGE_OFFSET in order to obtain a memory map of
* the virtual cpu's hardware control block.
*/
- self->sie_block = (struct kvm_s390_sie_block *)mmap(NULL, PAGE_SIZE,
- PROT_READ | PROT_WRITE, MAP_SHARED,
- self->vcpu_fd, KVM_S390_SIE_PAGE_OFFSET << PAGE_SHIFT);
- ASSERT_NE(self->sie_block, MAP_FAILED);
+ self->sie_block = __kvm_mmap(PAGE_SIZE, PROT_READ | PROT_WRITE,
+ MAP_SHARED, self->vcpu_fd,
+ KVM_S390_SIE_PAGE_OFFSET << PAGE_SHIFT);
TH_LOG("VM created %p %p", self->run, self->sie_block);
@@ -186,8 +184,8 @@ FIXTURE_SETUP(uc_kvm)
FIXTURE_TEARDOWN(uc_kvm)
{
- munmap(self->sie_block, PAGE_SIZE);
- munmap(self->run, self->kvm_run_size);
+ kvm_munmap(self->sie_block, PAGE_SIZE);
+ kvm_munmap(self->run, self->kvm_run_size);
close(self->vcpu_fd);
close(self->vm_fd);
close(self->kvm_fd);
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index ce3ac0fd6dfb..7fe427ff9b38 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -433,10 +433,10 @@ static void test_add_max_memory_regions(void)
pr_info("Adding slots 0..%i, each memory region with %dK size\n",
(max_mem_slots - 1), MEM_REGION_SIZE >> 10);
- mem = mmap(NULL, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment,
- PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
- TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
+
+ mem = kvm_mmap((size_t)max_mem_slots * MEM_REGION_SIZE + alignment,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1);
mem_aligned = (void *)(((size_t) mem + alignment - 1) & ~(alignment - 1));
for (slot = 0; slot < max_mem_slots; slot++)
@@ -446,9 +446,8 @@ static void test_add_max_memory_regions(void)
mem_aligned + (uint64_t)slot * MEM_REGION_SIZE);
/* Check it cannot be added memory slots beyond the limit */
- mem_extra = mmap(NULL, MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- TEST_ASSERT(mem_extra != MAP_FAILED, "Failed to mmap() host");
+ mem_extra = kvm_mmap(MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1);
ret = __vm_set_user_memory_region(vm, max_mem_slots, 0,
(uint64_t)max_mem_slots * MEM_REGION_SIZE,
@@ -456,8 +455,8 @@ static void test_add_max_memory_regions(void)
TEST_ASSERT(ret == -1 && errno == EINVAL,
"Adding one more memory slot should fail with EINVAL");
- munmap(mem, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment);
- munmap(mem_extra, MEM_REGION_SIZE);
+ kvm_munmap(mem, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment);
+ kvm_munmap(mem_extra, MEM_REGION_SIZE);
kvm_vm_free(vm);
}
--
2.51.0.536.g15c5d4f767-googSean Christopherson <seanjc@google.com> writes:
> Add and use wrappers for mmap() and munmap() that assert success to reduce
> a significant amount of boilerplate code, to ensure all tests assert on
> failure, and to provide consistent error messages on failure.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> .../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
> .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
> tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
> tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
> .../selftests/kvm/s390/ucontrol_test.c | 16 +++----
> .../selftests/kvm/set_memory_region_test.c | 17 ++++---
> 6 files changed, 64 insertions(+), 64 deletions(-)
>
>
> [...snip...]
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 23a506d7eca3..1c68ff0fb3fb 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
> #define __KVM_SYSCALL_ERROR(_name, _ret) \
> "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
>
> +static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
> + off_t offset)
Do you have a policy/rationale for putting this in kvm_util.h as opposed
to test_util.h? I like the idea of this wrapper but I thought this is
less of a kvm thing and more of a test utility, and hence it belongs in
test_util.c and test_util.h.
Also, the name kind of associates mmap with KVM too closely IMO, but
test_mmap() is not a great name either.
No strong opinions here.
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> +{
> + void *mem;
> +
> + mem = mmap(NULL, size, prot, flags, fd, offset);
> + TEST_ASSERT(mem != MAP_FAILED, __KVM_SYSCALL_ERROR("mmap()",
> + (int)(unsigned long)MAP_FAILED));
> +
> + return mem;
> +}
> +
> +static inline void *kvm_mmap(size_t size, int prot, int flags, int fd)
> +{
> + return __kvm_mmap(size, prot, flags, fd, 0);
> +}
> +
>
> [...snip...]
>
On Mon, Sep 29, 2025, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
>
> > Add and use wrappers for mmap() and munmap() that assert success to reduce
> > a significant amount of boilerplate code, to ensure all tests assert on
> > failure, and to provide consistent error messages on failure.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > .../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
> > .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
> > tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
> > tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
> > .../selftests/kvm/s390/ucontrol_test.c | 16 +++----
> > .../selftests/kvm/set_memory_region_test.c | 17 ++++---
> > 6 files changed, 64 insertions(+), 64 deletions(-)
> >
> >
> > [...snip...]
> >
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 23a506d7eca3..1c68ff0fb3fb 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
> > #define __KVM_SYSCALL_ERROR(_name, _ret) \
> > "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
> >
> > +static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
> > + off_t offset)
>
> Do you have a policy/rationale for putting this in kvm_util.h as opposed
> to test_util.h? I like the idea of this wrapper but I thought this is
> less of a kvm thing and more of a test utility, and hence it belongs in
> test_util.c and test_util.h.
To be perfectly honest, I forgot test_util.h existed :-)
> Also, the name kind of associates mmap with KVM too closely IMO, but
> test_mmap() is not a great name either.
Which file will hopefully be irrevelant, because ideally it'll be temporary (see
below). But if someone has a strong opinion and/or better idea on the name prefix,
I definitely want to settle on a name for syscall wrappers, because I want to go
much further than just adding an mmap() wrapper. I chose kvm_ because there's
basically zero chance that will ever conflict with generic selftests functionality,
and the wrappers utilize TEST_ASSERT(), which are unique to KVM selftests.
As for why the current location will hopefully be temporary, and why I want to
settle on a name, I have patches to add several more wrappers, along with
infrastructure to make it super easy to add new wrappers. When trying to sort
out the libnuma stuff for Shivank's series[*], I discovered that KVM selftests
already has a (very partial, very crappy) libnuma equivalent in
tools/testing/selftests/kvm/include/numaif.h.
Adding wrappers for NUMA syscalls became an exercise in frustration (so much
uninteresting boilerplate, and I kept making silly mistakes), and so that combined
with the desire for mmap() and munmap() wrappers motivated me to add a macro
framework similar to the kernel's DEFINE_SYSCALL magic.
So, I've got patches (that I'll post with the next version of the gmem NUMA
series) that add tools/testing/selftests/kvm/include/kvm_syscalls.h, and
__kvm_mmap() will be moved there (ideally it wouldn't move, but I want to land
this small series in 6.18, and so wanted to keep the changes for 6.18 small-ish).
For lack of a better namespace, and because we already have __KVM_SYSCALL_ERROR(),
I picked KVM_SYSCALL_DEFINE() for the "standard" builder, e.g. libnuma equivalents,
and then __KVM_SYSCALL_DEFINE() for a KVM selftests specific version to handle
asserting success.
/* Define a kvm_<syscall>() API to assert success. */
#define __KVM_SYSCALL_DEFINE(name, nr_args, args...) \
static inline void kvm_##name(DECLARE_ARGS(nr_args, args)) \
{ \
int r; \
\
r = name(UNPACK_ARGS(nr_args, args)); \
TEST_ASSERT(!r, __KVM_SYSCALL_ERROR(#name, r)); \
}
/*
* Macro to define syscall APIs, either because KVM selftests doesn't link to
* the standard library, e.g. libnuma, or because there is no library that yet
* provides the syscall. These
*/
#define KVM_SYSCALL_DEFINE(name, nr_args, args...) \
static inline long name(DECLARE_ARGS(nr_args, args)) \
{ \
return syscall(__NR_##name, UNPACK_ARGS(nr_args, args)); \
} \
__KVM_SYSCALL_DEFINE(name, nr_args, args)
The usage looks like this (which is odd at first glance, but makes it trivially
easy to copy+paste from the kernel SYSCALL_DEFINE invocations:
KVM_SYSCALL_DEFINE(get_mempolicy, 5, int *, policy, const unsigned long *, nmask,
unsigned long, maxnode, void *, addr, int, flags);
KVM_SYSCALL_DEFINE(set_mempolicy, 3, int, mode, const unsigned long *, nmask,
unsigned long, maxnode);
KVM_SYSCALL_DEFINE(set_mempolicy_home_node, 4, unsigned long, start,
unsigned long, len, unsigned long, home_node,
unsigned long, flags);
KVM_SYSCALL_DEFINE(migrate_pages, 4, int, pid, unsigned long, maxnode,
const unsigned long *, frommask, const unsigned long *, tomask);
KVM_SYSCALL_DEFINE(move_pages, 6, int, pid, unsigned long, count, void *, pages,
const int *, nodes, int *, status, int, flags);
KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, size, int, mode,
const unsigned long *, nodemask, unsigned long, maxnode,
unsigned int, flags);
__KVM_SYSCALL_DEFINE(munmap, 2, void *, mem, size_t, size);
__KVM_SYSCALL_DEFINE(close, 1, int, fd);
__KVM_SYSCALL_DEFINE(fallocate, 4, int, fd, int, mode, loff_t, offset, loff_t, len);
__KVM_SYSCALL_DEFINE(ftruncate, 2, unsigned int, fd, off_t, length);
[*] https://lore.kernel.org/all/0e986bdb-7d1b-4c14-932e-771a87532947@amd.com
Sean Christopherson <seanjc@google.com> writes:
> On Mon, Sep 29, 2025, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>
>> > Add and use wrappers for mmap() and munmap() that assert success to reduce
>> > a significant amount of boilerplate code, to ensure all tests assert on
>> > failure, and to provide consistent error messages on failure.
>> >
>> > No functional change intended.
>> >
>> > Signed-off-by: Sean Christopherson <seanjc@google.com>
>> > ---
>> > .../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
>> > .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
>> > tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
>> > tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
>> > .../selftests/kvm/s390/ucontrol_test.c | 16 +++----
>> > .../selftests/kvm/set_memory_region_test.c | 17 ++++---
>> > 6 files changed, 64 insertions(+), 64 deletions(-)
>> >
>> >
>> > [...snip...]
>> >
>> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
>> > index 23a506d7eca3..1c68ff0fb3fb 100644
>> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
>> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>> > @@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
>> > #define __KVM_SYSCALL_ERROR(_name, _ret) \
>> > "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
>> >
>> > +static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
>> > + off_t offset)
>>
>> Do you have a policy/rationale for putting this in kvm_util.h as opposed
>> to test_util.h? I like the idea of this wrapper but I thought this is
>> less of a kvm thing and more of a test utility, and hence it belongs in
>> test_util.c and test_util.h.
>
> To be perfectly honest, I forgot test_util.h existed :-)
>
Merging/dropping one of kvm_util.h vs test_util.h is a good idea. The
distinction is not clear and it's already kind of messy between the two.
>> Also, the name kind of associates mmap with KVM too closely IMO, but
>> test_mmap() is not a great name either.
>
> Which file will hopefully be irrevelant, because ideally it'll be temporary (see
> below). But if someone has a strong opinion and/or better idea on the name prefix,
> I definitely want to settle on a name for syscall wrappers, because I want to go
> much further than just adding an mmap() wrapper. I chose kvm_ because there's
> basically zero chance that will ever conflict with generic selftests functionality,
> and the wrappers utilize TEST_ASSERT(), which are unique to KVM selftests.
>
> As for why the current location will hopefully be temporary, and why I want to
> settle on a name, I have patches to add several more wrappers, along with
> infrastructure to make it super easy to add new wrappers. When trying to sort
> out the libnuma stuff for Shivank's series[*], I discovered that KVM selftests
> already has a (very partial, very crappy) libnuma equivalent in
> tools/testing/selftests/kvm/include/numaif.h.
>
> Adding wrappers for NUMA syscalls became an exercise in frustration (so much
> uninteresting boilerplate, and I kept making silly mistakes), and so that combined
> with the desire for mmap() and munmap() wrappers motivated me to add a macro
> framework similar to the kernel's DEFINE_SYSCALL magic.
>
> So, I've got patches (that I'll post with the next version of the gmem NUMA
> series) that add tools/testing/selftests/kvm/include/kvm_syscalls.h, and
> __kvm_mmap() will be moved there (ideally it wouldn't move, but I want to land
> this small series in 6.18, and so wanted to keep the changes for 6.18 small-ish).
>
> For lack of a better namespace, and because we already have __KVM_SYSCALL_ERROR(),
> I picked KVM_SYSCALL_DEFINE() for the "standard" builder, e.g. libnuma equivalents,
> and then __KVM_SYSCALL_DEFINE() for a KVM selftests specific version to handle
> asserting success.
>
It's a common pattern in KVM selftests to have a syscall/ioctl wrapper
foo() that asserts defaults and a __foo() that doesn't assert anything
and allows tests to assert something else, but I have a contrary
opinion.
I think it's better that tests be explicit about what they're testing
for, so perhaps it's better to use macros like TEST_ASSERT_EQ() to
explicitly call a function and check the results.
Or perhaps it should be more explicit, like in the name, that an
assertion is made within this function?
In many cases a foo() exists without the corresponding __foo(), which
seems to be discouraging testing for error cases.
Also, I guess especially for vcpu_run(), tests would like to loop/take
different actions based on different errnos and then it gets a bit
unwieldy to have to avoid functions that have assertions within them.
I can see people forgetting to add TEST_ASSERT_EQ()s to check results of
setup/teardown functions but I think those errors would surface some
other way anyway.
Not a strongly-held opinion, and no major concerns on the naming
either. It's a selftest after all and IIUC we're okay to have selftest
interfaces change anyway?
> /* Define a kvm_<syscall>() API to assert success. */
> #define __KVM_SYSCALL_DEFINE(name, nr_args, args...) \
> static inline void kvm_##name(DECLARE_ARGS(nr_args, args)) \
> { \
> int r; \
> \
> r = name(UNPACK_ARGS(nr_args, args)); \
> TEST_ASSERT(!r, __KVM_SYSCALL_ERROR(#name, r)); \
> }
>
> /*
> * Macro to define syscall APIs, either because KVM selftests doesn't link to
> * the standard library, e.g. libnuma, or because there is no library that yet
> * provides the syscall. These
> */
> #define KVM_SYSCALL_DEFINE(name, nr_args, args...) \
> static inline long name(DECLARE_ARGS(nr_args, args)) \
> { \
> return syscall(__NR_##name, UNPACK_ARGS(nr_args, args)); \
> } \
> __KVM_SYSCALL_DEFINE(name, nr_args, args)
>
>
> The usage looks like this (which is odd at first glance, but makes it trivially
> easy to copy+paste from the kernel SYSCALL_DEFINE invocations:
>
> KVM_SYSCALL_DEFINE(get_mempolicy, 5, int *, policy, const unsigned long *, nmask,
> unsigned long, maxnode, void *, addr, int, flags);
>
> KVM_SYSCALL_DEFINE(set_mempolicy, 3, int, mode, const unsigned long *, nmask,
> unsigned long, maxnode);
>
> KVM_SYSCALL_DEFINE(set_mempolicy_home_node, 4, unsigned long, start,
> unsigned long, len, unsigned long, home_node,
> unsigned long, flags);
>
> KVM_SYSCALL_DEFINE(migrate_pages, 4, int, pid, unsigned long, maxnode,
> const unsigned long *, frommask, const unsigned long *, tomask);
>
> KVM_SYSCALL_DEFINE(move_pages, 6, int, pid, unsigned long, count, void *, pages,
> const int *, nodes, int *, status, int, flags);
>
> KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, size, int, mode,
> const unsigned long *, nodemask, unsigned long, maxnode,
> unsigned int, flags);
>
> __KVM_SYSCALL_DEFINE(munmap, 2, void *, mem, size_t, size);
> __KVM_SYSCALL_DEFINE(close, 1, int, fd);
> __KVM_SYSCALL_DEFINE(fallocate, 4, int, fd, int, mode, loff_t, offset, loff_t, len);
> __KVM_SYSCALL_DEFINE(ftruncate, 2, unsigned int, fd, off_t, length);
>
> [*] https://lore.kernel.org/all/0e986bdb-7d1b-4c14-932e-771a87532947@amd.com
On Tue, Sep 30, 2025, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > To be perfectly honest, I forgot test_util.h existed :-)
>
> Merging/dropping one of kvm_util.h vs test_util.h is a good idea. The
> distinction is not clear and it's already kind of messy between the two.
That's a topic for another day.
> It's a common pattern in KVM selftests to have a syscall/ioctl wrapper
> foo() that asserts defaults and a __foo() that doesn't assert anything
> and allows tests to assert something else, but I have a contrary
> opinion.
>
> I think it's better that tests be explicit about what they're testing
> for, so perhaps it's better to use macros like TEST_ASSERT_EQ() to
> explicitly call a function and check the results.
No, foo() and __foo() is a well-established pattern in the kernel, and in KVM
selftests it is a very well-established pattern for syscalls and ioctls. And
I feel very, very strong about handling errors in the core infrastructure.
Relying on developers to remember to add an assert is 100% guaranteed to result
in missed asserts. That makes everyone's life painful, because inevitably an
ioctl will fail on someone else's system, and then they're stuck debugging a
super random failure with no insight into what the developer _meant_ to do.
And requiring developers to write (i.e. copy+paste) boring, uninteresting code
to handle failures adds a lot of friction to development, is a terrible use of
developers' time, and results in _awful_ error messages. Bad or missing error
messages in tests have easily wasted tens of hours of just _my_ time; I suspect
the total cost throughout the KVM community can be measured in tens of days.
E.g. pop quiz, what state did I clobber that generated this error message with
a TEST_ASSERT_EQ(ret, 0)? Answer at the bottom.
==== Test Assertion Failure ====
lib/x86/processor.c:1128: ret == 0
pid=2456 tid=2456 errno=22 - Invalid argument
1 0x0000000000415465: vcpu_load_state at processor.c:1128
2 0x0000000000402805: save_restore_vm at hyperv_evmcs.c:221
3 0x000000000040204d: main at hyperv_evmcs.c:286
4 0x000000000041df43: __libc_start_call_main at libc-start.o:?
5 0x00000000004200ec: __libc_start_main_impl at ??:?
6 0x0000000000402220: _start at ??:?
0xffffffffffffffff != 0 (ret != 0)
You might say "oh, I can go look at the source". But what if you don't have the
source because you got a test failure from CI? Or because the assert came from
a bug report due to a failure in someone else's CI pipeline?
That is not a contrived example. Before the ioctl assertion framework was added,
KVM selftests was littered with such garbage. Note, I'm not blaming developers
in any way. After having to add tens of asserts on KVM ioctls just to write a
simple test, it's entirely natural to become fatigued and start throwing in
TEST_ASSERT_EQ(ret, 0) or TEST_ASSERT(!ret, "ioctl failed").
There's also the mechanics of requiring the caller to assert. KVM ioctls that
return a single value, e.g. register accessors, then need to use an out-param to
communicate the value or error code, e.g. this
val = vcpu_get_reg(vcpu, reg_id);
TEST_ASSERT_EQ(val, 0);
would become this:
ret = vcpu_get_reg(vcpu, reg_id, &val);
TEST_ASSERT_EQ(ret, 0);
TEST_ASSERT_EQ(val, 0);
But of course, the developer would bundle that into:
TEST_ASSERT(!ret && !val, "get_reg failed");
And then the user is really sad when the "!val" condition fails, because they
can't even tell. Again, this't a contrived example, it literally happend to me
when dealing with the guest_memfd NUMA testcase, and was what prompted me to
write this syscall framework. This also shows the typical error message that a
developer will write.
This TEST_ASSERT() failed on me due to a misguided cleanup I made:
ret = syscall(__NR_get_mempolicy, &get_policy, &get_nodemask,
maxnode, mem, MPOL_F_ADDR);
TEST_ASSERT(!ret && get_policy == MPOL_DEFAULT && get_nodemask == 0,
"Policy should be MPOL_DEFAULT and nodes zero");
generating this error message:
==== Test Assertion Failure ====
guest_memfd_test.c:120: !ret && get_policy == MPOL_DEFAULT && get_nodemask == 0
pid=52062 tid=52062 errno=22 - Invalid argument
1 0x0000000000404113: test_mbind at guest_memfd_test.c:120 (discriminator 6)
2 (inlined by) __test_guest_memfd at guest_memfd_test.c:409 (discriminator 6)
3 0x0000000000402320: test_guest_memfd at guest_memfd_test.c:432
4 (inlined by) main at guest_memfd_test.c:529
5 0x000000000041eda3: __libc_start_call_main at libc-start.o:?
6 0x0000000000420f4c: __libc_start_main_impl at ??:?
7 0x00000000004025c0: _start at ??:?
Policy should be MPOL_DEFAULT and nodes zero
At first glance, it would appear that get_mempolicy() failed with -EINVAL. Nope.
ret==0, but errno was left set from an earlier syscall. It took me a few minutes
of digging and a run with strace to figure out that get_mempolicy() succeeded.
Constrast that with:
kvm_get_mempolicy(&policy, &nodemask, maxnode, mem, MPOL_F_ADDR);
TEST_ASSERT(policy == MPOL_DEFAULT && !nodemask,
"Wanted MPOL_DEFAULT (%u) and nodemask 0x0, got %u and 0x%lx",
MPOL_DEFAULT, policy, nodemask);
==== Test Assertion Failure ====
guest_memfd_test.c:120: policy == MPOL_DEFAULT && !nodemask
pid=52700 tid=52700 errno=22 - Invalid argument
1 0x0000000000404915: test_mbind at guest_memfd_test.c:120 (discriminator 6)
2 (inlined by) __test_guest_memfd at guest_memfd_test.c:407 (discriminator 6)
3 0x0000000000402320: test_guest_memfd at guest_memfd_test.c:430
4 (inlined by) main at guest_memfd_test.c:527
5 0x000000000041eda3: __libc_start_call_main at libc-start.o:?
6 0x0000000000420f4c: __libc_start_main_impl at ??:?
7 0x00000000004025c0: _start at ??:?
Wanted MPOL_DEFAULT (0) and nodemask 0x0, got 1 and 0x1
Yeah, there's still some noise with errno=22, but it's fairly clear that the
returned values mismatches, and super obvious that the syscall succeeded when
looking at the code. This is not a cherry-picked example. There are hundreds,
if not thousands, of such asserts in KVM selftests and KVM-Unit-Tests in
particular. And that's when developers _aren't_ forced to manually add boilerplate
asserts in ioctls succeeding.
For people that are completely new to KVM selftests, I can appreciate that it
might take a while to acclimate to the foo() and __foo() pattern, but I have a
hard time believing that it adds significant cognitive load after you've spent
a decent amount of time in KVM selftests. And I 100% want to cater to the people
that are dealing with KVM selftests day in, day out.
> Or perhaps it should be more explicit, like in the name, that an
> assertion is made within this function?
No, that's entirely inflexible, will lead to confusion, and adds a copious amount
of noise. E.g. this
/* emulate hypervisor clearing CR4.OSXSAVE */
vcpu_sregs_get(vcpu, &sregs);
sregs.cr4 &= ~X86_CR4_OSXSAVE;
vcpu_sregs_set(vcpu, &sregs);
versus
/* emulate hypervisor clearing CR4.OSXSAVE */
vcpu_sregs_get_assert(vcpu, &sregs);
sregs.cr4 &= ~X86_CR4_OSXSAVE;
vcpu_sregs_set_assert(vcpu, &sregs);
The "assert" is pure noise and makes it harder to see the "get" versus "set".
If we instead annotate the the "no_assert" case, then we'll end up with ambigous
cases where a developer won't be able to determine if an unannotated API asserts
or not, and conflict cases where a "no_assert" API _does_ assert, just not on the
primary ioctl it's invoking.
IMO, foo() and __foo() is quite explicit once you become accustomed to the
environment.
> In many cases a foo() exists without the corresponding __foo(), which
> seems to be discouraging testing for error cases.
That's almost always because no one has needed __foo().
> Also, I guess especially for vcpu_run(), tests would like to loop/take
> different actions based on different errnos and then it gets a bit
> unwieldy to have to avoid functions that have assertions within them.
vcpu_run() is a special case. KVM_RUN is so much more than a normal ioctl, and
so having vcpu_run() follow the "standard" pattern isn't entirely feasible.
Speaking of vcpu_run(), and directly related to idea of having developers manually
do TEST_ASSERT_EQ(), one of the top items on my selftests todo list is to have
vcpu_run() handle GUEST_ASSERT and GUEST_PRINTF whenever possible. Having to add
UCALL_PRINTF handling just to get a debug message out of a test's guest code is
beyond frustrating. Ditto for the 60+ tests that had to manually add UCALL_ABORT
handling, which leads to tests having code like this, which then gets copy+pasted
all over the place and becomes a nightmare to maintain.
static void __vcpu_run_expect(struct kvm_vcpu *vcpu, unsigned int cmd)
{
struct ucall uc;
vcpu_run(vcpu);
switch (get_ucall(vcpu, &uc)) {
case UCALL_ABORT:
REPORT_GUEST_ASSERT(uc);
break;
default:
if (uc.cmd == cmd)
return;
TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
}
}
> I can see people forgetting to add TEST_ASSERT_EQ()s to check results of
> setup/teardown functions but I think those errors would surface some
> other way anyway.
Heh, I don't mean to be condescending, but I highly doubt you'll have this
opinion after you've had to debug a completely unfamiliar test that's failing
in weird ways, for the tenth time.
> Not a strongly-held opinion,
As you may have noticed, I have extremely strong opinions in this area :-)
> and no major concerns on the naming either. It's a selftest after all and
> IIUC we're okay to have selftest interfaces change anyway?
Yes, changes are fine. It's the churn I want to avoid.
Oh, and here's the "answer" to the TEST_ASSERT_EQ() failure:
==== Test Assertion Failure ====
include/kvm_util.h:794: !ret
pid=43866 tid=43866 errno=22 - Invalid argument
1 0x0000000000415486: vcpu_sregs_set at kvm_util.h:794 (discriminator 4)
2 (inlined by) vcpu_load_state at processor.c:1125 (discriminator 4)
3 0x0000000000402805: save_restore_vm at hyperv_evmcs.c:221
4 0x000000000040204d: main at hyperv_evmcs.c:286
5 0x000000000041dfc3: __libc_start_call_main at libc-start.o:?
6 0x000000000042016c: __libc_start_main_impl at ??:?
7 0x0000000000402220: _start at ??:?
KVM_SET_SREGS failed, rc: -1 errno: 22 (Invalid argument)
Sean Christopherson <seanjc@google.com> writes:
> On Tue, Sep 30, 2025, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> > To be perfectly honest, I forgot test_util.h existed :-)
>>
>> Merging/dropping one of kvm_util.h vs test_util.h is a good idea. The
>> distinction is not clear and it's already kind of messy between the two.
>
> That's a topic for another day.
>
>> It's a common pattern in KVM selftests to have a syscall/ioctl wrapper
>> foo() that asserts defaults and a __foo() that doesn't assert anything
>> and allows tests to assert something else, but I have a contrary
>> opinion.
>>
>> I think it's better that tests be explicit about what they're testing
>> for, so perhaps it's better to use macros like TEST_ASSERT_EQ() to
>> explicitly call a function and check the results.
>
> No, foo() and __foo() is a well-established pattern in the kernel, and in KVM
> selftests it is a very well-established pattern for syscalls and ioctls. And
> I feel very, very strong about handling errors in the core infrastructure.
>
> Relying on developers to remember to add an assert is 100% guaranteed to result
> in missed asserts. That makes everyone's life painful, because inevitably an
> ioctl will fail on someone else's system, and then they're stuck debugging a
> super random failure with no insight into what the developer _meant_ to do.
>
> And requiring developers to write (i.e. copy+paste) boring, uninteresting code
> to handle failures adds a lot of friction to development, is a terrible use of
> developers' time, and results in _awful_ error messages. Bad or missing error
> messages in tests have easily wasted tens of hours of just _my_ time; I suspect
> the total cost throughout the KVM community can be measured in tens of days.
>
> E.g. pop quiz, what state did I clobber that generated this error message with
> a TEST_ASSERT_EQ(ret, 0)? Answer at the bottom.
>
> ==== Test Assertion Failure ====
> lib/x86/processor.c:1128: ret == 0
> pid=2456 tid=2456 errno=22 - Invalid argument
> 1 0x0000000000415465: vcpu_load_state at processor.c:1128
> 2 0x0000000000402805: save_restore_vm at hyperv_evmcs.c:221
> 3 0x000000000040204d: main at hyperv_evmcs.c:286
> 4 0x000000000041df43: __libc_start_call_main at libc-start.o:?
> 5 0x00000000004200ec: __libc_start_main_impl at ??:?
> 6 0x0000000000402220: _start at ??:?
> 0xffffffffffffffff != 0 (ret != 0)
>
> You might say "oh, I can go look at the source". But what if you don't have the
> source because you got a test failure from CI? Or because the assert came from
> a bug report due to a failure in someone else's CI pipeline?
>
> That is not a contrived example. Before the ioctl assertion framework was added,
> KVM selftests was littered with such garbage. Note, I'm not blaming developers
> in any way. After having to add tens of asserts on KVM ioctls just to write a
> simple test, it's entirely natural to become fatigued and start throwing in
> TEST_ASSERT_EQ(ret, 0) or TEST_ASSERT(!ret, "ioctl failed").
>
> There's also the mechanics of requiring the caller to assert. KVM ioctls that
> return a single value, e.g. register accessors, then need to use an out-param to
> communicate the value or error code, e.g. this
>
> val = vcpu_get_reg(vcpu, reg_id);
> TEST_ASSERT_EQ(val, 0);
>
> would become this:
>
> ret = vcpu_get_reg(vcpu, reg_id, &val);
> TEST_ASSERT_EQ(ret, 0);
> TEST_ASSERT_EQ(val, 0);
>
> But of course, the developer would bundle that into:
>
> TEST_ASSERT(!ret && !val, "get_reg failed");
>
> And then the user is really sad when the "!val" condition fails, because they
> can't even tell. Again, this't a contrived example, it literally happend to me
> when dealing with the guest_memfd NUMA testcase, and was what prompted me to
> write this syscall framework. This also shows the typical error message that a
> developer will write.
>
> This TEST_ASSERT() failed on me due to a misguided cleanup I made:
>
> ret = syscall(__NR_get_mempolicy, &get_policy, &get_nodemask,
> maxnode, mem, MPOL_F_ADDR);
> TEST_ASSERT(!ret && get_policy == MPOL_DEFAULT && get_nodemask == 0,
> "Policy should be MPOL_DEFAULT and nodes zero");
>
> generating this error message:
>
> ==== Test Assertion Failure ====
> guest_memfd_test.c:120: !ret && get_policy == MPOL_DEFAULT && get_nodemask == 0
> pid=52062 tid=52062 errno=22 - Invalid argument
> 1 0x0000000000404113: test_mbind at guest_memfd_test.c:120 (discriminator 6)
> 2 (inlined by) __test_guest_memfd at guest_memfd_test.c:409 (discriminator 6)
> 3 0x0000000000402320: test_guest_memfd at guest_memfd_test.c:432
> 4 (inlined by) main at guest_memfd_test.c:529
> 5 0x000000000041eda3: __libc_start_call_main at libc-start.o:?
> 6 0x0000000000420f4c: __libc_start_main_impl at ??:?
> 7 0x00000000004025c0: _start at ??:?
> Policy should be MPOL_DEFAULT and nodes zero
>
> At first glance, it would appear that get_mempolicy() failed with -EINVAL. Nope.
> ret==0, but errno was left set from an earlier syscall. It took me a few minutes
> of digging and a run with strace to figure out that get_mempolicy() succeeded.
>
> Constrast that with:
>
> kvm_get_mempolicy(&policy, &nodemask, maxnode, mem, MPOL_F_ADDR);
> TEST_ASSERT(policy == MPOL_DEFAULT && !nodemask,
> "Wanted MPOL_DEFAULT (%u) and nodemask 0x0, got %u and 0x%lx",
> MPOL_DEFAULT, policy, nodemask);
>
> ==== Test Assertion Failure ====
> guest_memfd_test.c:120: policy == MPOL_DEFAULT && !nodemask
> pid=52700 tid=52700 errno=22 - Invalid argument
> 1 0x0000000000404915: test_mbind at guest_memfd_test.c:120 (discriminator 6)
> 2 (inlined by) __test_guest_memfd at guest_memfd_test.c:407 (discriminator 6)
> 3 0x0000000000402320: test_guest_memfd at guest_memfd_test.c:430
> 4 (inlined by) main at guest_memfd_test.c:527
> 5 0x000000000041eda3: __libc_start_call_main at libc-start.o:?
> 6 0x0000000000420f4c: __libc_start_main_impl at ??:?
> 7 0x00000000004025c0: _start at ??:?
> Wanted MPOL_DEFAULT (0) and nodemask 0x0, got 1 and 0x1
>
> Yeah, there's still some noise with errno=22, but it's fairly clear that the
> returned values mismatches, and super obvious that the syscall succeeded when
> looking at the code. This is not a cherry-picked example. There are hundreds,
> if not thousands, of such asserts in KVM selftests and KVM-Unit-Tests in
> particular. And that's when developers _aren't_ forced to manually add boilerplate
> asserts in ioctls succeeding.
>
> For people that are completely new to KVM selftests, I can appreciate that it
> might take a while to acclimate to the foo() and __foo() pattern, but I have a
> hard time believing that it adds significant cognitive load after you've spent
> a decent amount of time in KVM selftests. And I 100% want to cater to the people
> that are dealing with KVM selftests day in, day out.
>
Thanks for taking the time to write this up. I'm going to start a list
of "most useful explanations" and this will go on that list.
>> Or perhaps it should be more explicit, like in the name, that an
>> assertion is made within this function?
>
> No, that's entirely inflexible, will lead to confusion, and adds a copious amount
> of noise. E.g. this
>
> /* emulate hypervisor clearing CR4.OSXSAVE */
> vcpu_sregs_get(vcpu, &sregs);
> sregs.cr4 &= ~X86_CR4_OSXSAVE;
> vcpu_sregs_set(vcpu, &sregs);
>
> versus
>
> /* emulate hypervisor clearing CR4.OSXSAVE */
> vcpu_sregs_get_assert(vcpu, &sregs);
> sregs.cr4 &= ~X86_CR4_OSXSAVE;
> vcpu_sregs_set_assert(vcpu, &sregs);
>
> The "assert" is pure noise and makes it harder to see the "get" versus "set".
>
> If we instead annotate the the "no_assert" case, then we'll end up with ambigous
> cases where a developer won't be able to determine if an unannotated API asserts
> or not, and conflict cases where a "no_assert" API _does_ assert, just not on the
> primary ioctl it's invoking.
>
> IMO, foo() and __foo() is quite explicit once you become accustomed to the
> environment.
>
>> In many cases a foo() exists without the corresponding __foo(), which
>> seems to be discouraging testing for error cases.
>
> That's almost always because no one has needed __foo().
>
>> Also, I guess especially for vcpu_run(), tests would like to loop/take
>> different actions based on different errnos and then it gets a bit
>> unwieldy to have to avoid functions that have assertions within them.
>
> vcpu_run() is a special case. KVM_RUN is so much more than a normal ioctl, and
> so having vcpu_run() follow the "standard" pattern isn't entirely feasible.
>
> Speaking of vcpu_run(), and directly related to idea of having developers manually
> do TEST_ASSERT_EQ(), one of the top items on my selftests todo list is to have
> vcpu_run() handle GUEST_ASSERT and GUEST_PRINTF whenever possible. Having to add
> UCALL_PRINTF handling just to get a debug message out of a test's guest code is
> beyond frustrating. Ditto for the 60+ tests that had to manually add UCALL_ABORT
> handling, which leads to tests having code like this, which then gets copy+pasted
> all over the place and becomes a nightmare to maintain.
+1000 this is exactly where I had to avoid assertions!
>
> static void __vcpu_run_expect(struct kvm_vcpu *vcpu, unsigned int cmd)
> {
> struct ucall uc;
>
> vcpu_run(vcpu);
> switch (get_ucall(vcpu, &uc)) {
> case UCALL_ABORT:
> REPORT_GUEST_ASSERT(uc);
> break;
> default:
> if (uc.cmd == cmd)
> return;
>
> TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
> }
> }
>
>> I can see people forgetting to add TEST_ASSERT_EQ()s to check results of
>> setup/teardown functions but I think those errors would surface some
>> other way anyway.
>
> Heh, I don't mean to be condescending, but I highly doubt you'll have this
> opinion after you've had to debug a completely unfamiliar test that's failing
> in weird ways, for the tenth time.
>
>> Not a strongly-held opinion,
>
> As you may have noticed, I have extremely strong opinions in this area :-)
>
>> and no major concerns on the naming either. It's a selftest after all and
>> IIUC we're okay to have selftest interfaces change anyway?
>
> Yes, changes are fine. It's the churn I want to avoid.
>
> Oh, and here's the "answer" to the TEST_ASSERT_EQ() failure:
>
> ==== Test Assertion Failure ====
> include/kvm_util.h:794: !ret
> pid=43866 tid=43866 errno=22 - Invalid argument
> 1 0x0000000000415486: vcpu_sregs_set at kvm_util.h:794 (discriminator 4)
> 2 (inlined by) vcpu_load_state at processor.c:1125 (discriminator 4)
> 3 0x0000000000402805: save_restore_vm at hyperv_evmcs.c:221
> 4 0x000000000040204d: main at hyperv_evmcs.c:286
> 5 0x000000000041dfc3: __libc_start_call_main at libc-start.o:?
> 6 0x000000000042016c: __libc_start_main_impl at ??:?
> 7 0x0000000000402220: _start at ??:?
> KVM_SET_SREGS failed, rc: -1 errno: 22 (Invalid argument)
On 26.09.25 18:31, Sean Christopherson wrote: > Add and use wrappers for mmap() and munmap() that assert success to reduce > a significant amount of boilerplate code, to ensure all tests assert on > failure, and to provide consistent error messages on failure. > > No functional change intended. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- Reviewed-by: David Hildenbrand <david@redhat.com> -- Cheers David / dhildenb
On Fri, 26 Sept 2025 at 17:31, Sean Christopherson <seanjc@google.com> wrote:
>
> Add and use wrappers for mmap() and munmap() that assert success to reduce
> a significant amount of boilerplate code, to ensure all tests assert on
> failure, and to provide consistent error messages on failure.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Tested-by: Fuad Tabba <tabba@google.com>
(except for the s390/ucontrol_test.c, which I didn't test)
Cheers,
/fuad
> ---
> .../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
> .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
> tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
> tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
> .../selftests/kvm/s390/ucontrol_test.c | 16 +++----
> .../selftests/kvm/set_memory_region_test.c | 17 ++++---
> 6 files changed, 64 insertions(+), 64 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 5a50a28ce1fa..5dd40b77dc07 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -50,8 +50,7 @@ static void test_mmap_supported(int fd, size_t total_size)
> mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> TEST_ASSERT(mem == MAP_FAILED, "Copy-on-write not allowed by guest_memfd.");
>
> - mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "mmap() for guest_memfd should succeed.");
> + mem = kvm_mmap(total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> memset(mem, val, total_size);
> for (i = 0; i < total_size; i++)
> @@ -70,8 +69,7 @@ static void test_mmap_supported(int fd, size_t total_size)
> for (i = 0; i < total_size; i++)
> TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
> - ret = munmap(mem, total_size);
> - TEST_ASSERT(!ret, "munmap() should succeed.");
> + kvm_munmap(mem, total_size);
> }
>
> static sigjmp_buf jmpbuf;
> @@ -89,10 +87,8 @@ static void test_fault_overflow(int fd, size_t total_size)
> const char val = 0xaa;
> char *mem;
> size_t i;
> - int ret;
>
> - mem = mmap(NULL, map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "mmap() for guest_memfd should succeed.");
> + mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> sigaction(SIGBUS, &sa_new, &sa_old);
> if (sigsetjmp(jmpbuf, 1) == 0) {
> @@ -104,8 +100,7 @@ static void test_fault_overflow(int fd, size_t total_size)
> for (i = 0; i < total_size; i++)
> TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
> - ret = munmap(mem, map_size);
> - TEST_ASSERT(!ret, "munmap() should succeed.");
> + kvm_munmap(mem, map_size);
> }
>
> static void test_mmap_not_supported(int fd, size_t total_size)
> @@ -347,10 +342,9 @@ static void test_guest_memfd_guest(void)
> GUEST_MEMFD_FLAG_DEFAULT_SHARED);
> vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, size, NULL, fd, 0);
>
> - mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "mmap() on guest_memfd failed");
> + mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> memset(mem, 0xaa, size);
> - munmap(mem, size);
> + kvm_munmap(mem, size);
>
> virt_pg_map(vm, gpa, gpa);
> vcpu_args_set(vcpu, 2, gpa, size);
> @@ -358,8 +352,7 @@ static void test_guest_memfd_guest(void)
>
> TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
>
> - mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "mmap() on guest_memfd failed");
> + mem = kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
> for (i = 0; i < size; i++)
> TEST_ASSERT_EQ(mem[i], 0xff);
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 23a506d7eca3..1c68ff0fb3fb 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
> #define __KVM_SYSCALL_ERROR(_name, _ret) \
> "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
>
> +static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
> + off_t offset)
> +{
> + void *mem;
> +
> + mem = mmap(NULL, size, prot, flags, fd, offset);
> + TEST_ASSERT(mem != MAP_FAILED, __KVM_SYSCALL_ERROR("mmap()",
> + (int)(unsigned long)MAP_FAILED));
> +
> + return mem;
> +}
> +
> +static inline void *kvm_mmap(size_t size, int prot, int flags, int fd)
> +{
> + return __kvm_mmap(size, prot, flags, fd, 0);
> +}
> +
> +static inline void kvm_munmap(void *mem, size_t size)
> +{
> + int ret;
> +
> + ret = munmap(mem, size);
> + TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
> +}
> +
> /*
> * Use the "inner", double-underscore macro when reporting errors from within
> * other macros so that the name of ioctl() and not its literal numeric value
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index c3f5142b0a54..da754b152c11 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -770,13 +770,11 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> int ret;
>
> if (vcpu->dirty_gfns) {
> - ret = munmap(vcpu->dirty_gfns, vm->dirty_ring_size);
> - TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
> + kvm_munmap(vcpu->dirty_gfns, vm->dirty_ring_size);
> vcpu->dirty_gfns = NULL;
> }
>
> - ret = munmap(vcpu->run, vcpu_mmap_sz());
> - TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
> + kvm_munmap(vcpu->run, vcpu_mmap_sz());
>
> ret = close(vcpu->fd);
> TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("close()", ret));
> @@ -810,20 +808,16 @@ void kvm_vm_release(struct kvm_vm *vmp)
> static void __vm_mem_region_delete(struct kvm_vm *vm,
> struct userspace_mem_region *region)
> {
> - int ret;
> -
> rb_erase(®ion->gpa_node, &vm->regions.gpa_tree);
> rb_erase(®ion->hva_node, &vm->regions.hva_tree);
> hash_del(®ion->slot_node);
>
> sparsebit_free(®ion->unused_phy_pages);
> sparsebit_free(®ion->protected_phy_pages);
> - ret = munmap(region->mmap_start, region->mmap_size);
> - TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
> + kvm_munmap(region->mmap_start, region->mmap_size);
> if (region->fd >= 0) {
> /* There's an extra map when using shared memory. */
> - ret = munmap(region->mmap_alias, region->mmap_size);
> - TEST_ASSERT(!ret, __KVM_SYSCALL_ERROR("munmap()", ret));
> + kvm_munmap(region->mmap_alias, region->mmap_size);
> close(region->fd);
> }
> if (region->region.guest_memfd >= 0)
> @@ -1080,12 +1074,9 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> region->fd = kvm_memfd_alloc(region->mmap_size,
> src_type == VM_MEM_SRC_SHARED_HUGETLB);
>
> - region->mmap_start = mmap(NULL, region->mmap_size,
> - PROT_READ | PROT_WRITE,
> - vm_mem_backing_src_alias(src_type)->flag,
> - region->fd, 0);
> - TEST_ASSERT(region->mmap_start != MAP_FAILED,
> - __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
> + region->mmap_start = kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE,
> + vm_mem_backing_src_alias(src_type)->flag,
> + region->fd);
>
> TEST_ASSERT(!is_backing_src_hugetlb(src_type) ||
> region->mmap_start == align_ptr_up(region->mmap_start, backing_src_pagesz),
> @@ -1156,12 +1147,10 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
>
> /* If shared memory, create an alias. */
> if (region->fd >= 0) {
> - region->mmap_alias = mmap(NULL, region->mmap_size,
> - PROT_READ | PROT_WRITE,
> - vm_mem_backing_src_alias(src_type)->flag,
> - region->fd, 0);
> - TEST_ASSERT(region->mmap_alias != MAP_FAILED,
> - __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
> + region->mmap_alias = kvm_mmap(region->mmap_size,
> + PROT_READ | PROT_WRITE,
> + vm_mem_backing_src_alias(src_type)->flag,
> + region->fd);
>
> /* Align host alias address */
> region->host_alias = align_ptr_up(region->mmap_alias, alignment);
> @@ -1371,10 +1360,8 @@ struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
> TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->run), "vcpu mmap size "
> "smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
> vcpu_mmap_sz(), sizeof(*vcpu->run));
> - vcpu->run = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
> - PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
> - TEST_ASSERT(vcpu->run != MAP_FAILED,
> - __KVM_SYSCALL_ERROR("mmap()", (int)(unsigned long)MAP_FAILED));
> + vcpu->run = kvm_mmap(vcpu_mmap_sz(), PROT_READ | PROT_WRITE,
> + MAP_SHARED, vcpu->fd);
>
> if (kvm_has_cap(KVM_CAP_BINARY_STATS_FD))
> vcpu->stats.fd = vcpu_get_stats_fd(vcpu);
> @@ -1821,9 +1808,8 @@ void *vcpu_map_dirty_ring(struct kvm_vcpu *vcpu)
> page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
> TEST_ASSERT(addr == MAP_FAILED, "Dirty ring mapped exec");
>
> - addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd,
> - page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
> - TEST_ASSERT(addr != MAP_FAILED, "Dirty ring map failed");
> + addr = __kvm_mmap(size, PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd,
> + page_size * KVM_DIRTY_LOG_PAGE_OFFSET);
>
> vcpu->dirty_gfns = addr;
> vcpu->dirty_gfns_count = size / sizeof(struct kvm_dirty_gfn);
> diff --git a/tools/testing/selftests/kvm/mmu_stress_test.c b/tools/testing/selftests/kvm/mmu_stress_test.c
> index 6a437d2be9fa..37b7e6524533 100644
> --- a/tools/testing/selftests/kvm/mmu_stress_test.c
> +++ b/tools/testing/selftests/kvm/mmu_stress_test.c
> @@ -339,8 +339,7 @@ int main(int argc, char *argv[])
> TEST_ASSERT(max_gpa > (4 * slot_size), "MAXPHYADDR <4gb ");
>
> fd = kvm_memfd_alloc(slot_size, hugepages);
> - mem = mmap(NULL, slot_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "mmap() failed");
> + mem = kvm_mmap(slot_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> TEST_ASSERT(!madvise(mem, slot_size, MADV_NOHUGEPAGE), "madvise() failed");
>
> @@ -413,7 +412,7 @@ int main(int argc, char *argv[])
> for (slot = (slot - 1) & ~1ull; slot >= first_slot; slot -= 2)
> vm_set_user_memory_region(vm, slot, 0, 0, 0, NULL);
>
> - munmap(mem, slot_size / 2);
> + kvm_munmap(mem, slot_size / 2);
>
> /* Sanity check that the vCPUs actually ran. */
> for (i = 0; i < nr_vcpus; i++)
> diff --git a/tools/testing/selftests/kvm/s390/ucontrol_test.c b/tools/testing/selftests/kvm/s390/ucontrol_test.c
> index d265b34c54be..50bc1c38225a 100644
> --- a/tools/testing/selftests/kvm/s390/ucontrol_test.c
> +++ b/tools/testing/selftests/kvm/s390/ucontrol_test.c
> @@ -142,19 +142,17 @@ FIXTURE_SETUP(uc_kvm)
> self->kvm_run_size = ioctl(self->kvm_fd, KVM_GET_VCPU_MMAP_SIZE, NULL);
> ASSERT_GE(self->kvm_run_size, sizeof(struct kvm_run))
> TH_LOG(KVM_IOCTL_ERROR(KVM_GET_VCPU_MMAP_SIZE, self->kvm_run_size));
> - self->run = (struct kvm_run *)mmap(NULL, self->kvm_run_size,
> - PROT_READ | PROT_WRITE, MAP_SHARED, self->vcpu_fd, 0);
> - ASSERT_NE(self->run, MAP_FAILED);
> + self->run = kvm_mmap(self->kvm_run_size, PROT_READ | PROT_WRITE,
> + MAP_SHARED, self->vcpu_fd);
> /**
> * For virtual cpus that have been created with S390 user controlled
> * virtual machines, the resulting vcpu fd can be memory mapped at page
> * offset KVM_S390_SIE_PAGE_OFFSET in order to obtain a memory map of
> * the virtual cpu's hardware control block.
> */
> - self->sie_block = (struct kvm_s390_sie_block *)mmap(NULL, PAGE_SIZE,
> - PROT_READ | PROT_WRITE, MAP_SHARED,
> - self->vcpu_fd, KVM_S390_SIE_PAGE_OFFSET << PAGE_SHIFT);
> - ASSERT_NE(self->sie_block, MAP_FAILED);
> + self->sie_block = __kvm_mmap(PAGE_SIZE, PROT_READ | PROT_WRITE,
> + MAP_SHARED, self->vcpu_fd,
> + KVM_S390_SIE_PAGE_OFFSET << PAGE_SHIFT);
>
> TH_LOG("VM created %p %p", self->run, self->sie_block);
>
> @@ -186,8 +184,8 @@ FIXTURE_SETUP(uc_kvm)
>
> FIXTURE_TEARDOWN(uc_kvm)
> {
> - munmap(self->sie_block, PAGE_SIZE);
> - munmap(self->run, self->kvm_run_size);
> + kvm_munmap(self->sie_block, PAGE_SIZE);
> + kvm_munmap(self->run, self->kvm_run_size);
> close(self->vcpu_fd);
> close(self->vm_fd);
> close(self->kvm_fd);
> diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
> index ce3ac0fd6dfb..7fe427ff9b38 100644
> --- a/tools/testing/selftests/kvm/set_memory_region_test.c
> +++ b/tools/testing/selftests/kvm/set_memory_region_test.c
> @@ -433,10 +433,10 @@ static void test_add_max_memory_regions(void)
> pr_info("Adding slots 0..%i, each memory region with %dK size\n",
> (max_mem_slots - 1), MEM_REGION_SIZE >> 10);
>
> - mem = mmap(NULL, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment,
> - PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
> - TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
> +
> + mem = kvm_mmap((size_t)max_mem_slots * MEM_REGION_SIZE + alignment,
> + PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1);
> mem_aligned = (void *)(((size_t) mem + alignment - 1) & ~(alignment - 1));
>
> for (slot = 0; slot < max_mem_slots; slot++)
> @@ -446,9 +446,8 @@ static void test_add_max_memory_regions(void)
> mem_aligned + (uint64_t)slot * MEM_REGION_SIZE);
>
> /* Check it cannot be added memory slots beyond the limit */
> - mem_extra = mmap(NULL, MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> - TEST_ASSERT(mem_extra != MAP_FAILED, "Failed to mmap() host");
> + mem_extra = kvm_mmap(MEM_REGION_SIZE, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1);
>
> ret = __vm_set_user_memory_region(vm, max_mem_slots, 0,
> (uint64_t)max_mem_slots * MEM_REGION_SIZE,
> @@ -456,8 +455,8 @@ static void test_add_max_memory_regions(void)
> TEST_ASSERT(ret == -1 && errno == EINVAL,
> "Adding one more memory slot should fail with EINVAL");
>
> - munmap(mem, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment);
> - munmap(mem_extra, MEM_REGION_SIZE);
> + kvm_munmap(mem, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment);
> + kvm_munmap(mem_extra, MEM_REGION_SIZE);
> kvm_vm_free(vm);
> }
>
> --
> 2.51.0.536.g15c5d4f767-goog
>
© 2016 - 2025 Red Hat, Inc.