[PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[David Hildenbrand]

Let's test some basic functionality using /dev/mem. These tests will
implicitly cover some PAT (Page Attribute Handling) handling on x86.

These tests will only run when /dev/mem access to the first two pages
in physical address space is possible and allowed; otherwise, the tests
are skipped.

On current x86-64 with PAT inside a VM, all tests pass:

	TAP version 13
	1..19
	ok 1 madvise(MADV_DONTNEED) should be disallowed
	ok 2 madvise(MADV_DONTNEED_LOCKED) should be disallowed
	ok 3 madvise(MADV_FREE) should be disallowed
	ok 4 madvise(MADV_WIPEONFORK) should be disallowed
	ok 5 madvise(MADV_COLD) should be disallowed
	ok 6 madvise(MADV_PAGEOUT) should be disallowed
	ok 7 madvise(MADV_POPULATE_READ) should be disallowed
	ok 8 madvise(MADV_POPULATE_WRITE) should be disallowed
	ok 9 munmap() splitting
	ok 10 mmap() after splitting
	ok 11 mremap(MREMAP_FIXED)
	ok 12 mremap() shrinking
	ok 13 mremap() growing should be disallowed
	ok 14 mprotect(PROT_NONE)
	ok 15 SIGSEGV expected
	ok 16 mprotect(PROT_READ)
	ok 17 SIGSEGV not expected
	ok 18 fork()
	ok 19 SIGSEGV in child not expected
	# Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0

However, we are able to trigger:

[   27.888251] x86/PAT: pfnmap:1790 freeing invalid memtype [mem 0x00000000-0x00000fff]

There are probably more things worth testing in the future, such as
MAP_PRIVATE handling. But this set of tests is sufficient to cover most of
the things we will rework regarding PAT handling.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---

On current mm-unstable, the MADV_POPULATE_READ test fails because
mm-unstable contains a patch [1] that must be dropped.

[1] https://lore.kernel.org/all/20250507154105.763088-2-p.antoniou@partner.samsung.com/

---
 tools/testing/selftests/mm/Makefile |   1 +
 tools/testing/selftests/mm/pfnmap.c | 278 ++++++++++++++++++++++++++++
 2 files changed, 279 insertions(+)
 create mode 100644 tools/testing/selftests/mm/pfnmap.c

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index ad4d6043a60f0..ae6f994d3add7 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -84,6 +84,7 @@ TEST_GEN_FILES += mremap_test
 TEST_GEN_FILES += mseal_test
 TEST_GEN_FILES += on-fault-limit
 TEST_GEN_FILES += pagemap_ioctl
+TEST_GEN_FILES += pfnmap
 TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += uffd-stress
diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
new file mode 100644
index 0000000000000..59be2f3221124
--- /dev/null
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Basic VM_PFNMAP tests relying on mmap() of '/dev/mem'
+ *
+ * Copyright 2025, Red Hat, Inc.
+ *
+ * Author(s): David Hildenbrand <david@redhat.com>
+ */
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <linux/mman.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+
+#include "../kselftest.h"
+#include "vm_util.h"
+
+static size_t pagesize;
+static int pagemap_fd;
+static int dev_mem_fd;
+static sigjmp_buf env;
+
+static void signal_handler(int sig)
+{
+	if (sig == SIGSEGV)
+		siglongjmp(env, 1);
+	siglongjmp(env, 2);
+}
+
+static void sense_support(void)
+{
+	char *addr, tmp;
+	int ret;
+
+	dev_mem_fd = open("/dev/mem", O_RDONLY);
+	if (dev_mem_fd < 0)
+		ksft_exit_skip("Cannot open '/dev/mem': %s\n", strerror(errno));
+
+	/* We'll require the first two pages throughout our tests ... */
+	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_skip("Cannot mmap '/dev/mem'");
+
+	/* ... and want to be able to read from them. */
+	ret = sigsetjmp(env, 1);
+	if (!ret) {
+		tmp = *addr + *(addr + pagesize);
+		asm volatile("" : "+r" (tmp));
+	}
+	if (ret)
+		ksft_exit_skip("Cannot read-access mmap'ed '/dev/mem'");
+
+	munmap(addr, pagesize * 2);
+}
+
+static void test_madvise(void)
+{
+#define INIT_ADVICE(nr) { nr, #nr}
+	const struct {
+		int nr;
+		const char *name;
+	} advices[] = {
+		INIT_ADVICE(MADV_DONTNEED),
+		INIT_ADVICE(MADV_DONTNEED_LOCKED),
+		INIT_ADVICE(MADV_FREE),
+		INIT_ADVICE(MADV_WIPEONFORK),
+		INIT_ADVICE(MADV_COLD),
+		INIT_ADVICE(MADV_PAGEOUT),
+		INIT_ADVICE(MADV_POPULATE_READ),
+		INIT_ADVICE(MADV_POPULATE_WRITE),
+	};
+	char *addr;
+	int ret, i;
+
+	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* All these advices must be rejected. */
+	for (i = 0; i < ARRAY_SIZE(advices); i++) {
+		ret = madvise(addr, pagesize, advices[i].nr);
+		ksft_test_result(ret && errno == EINVAL,
+				 "madvise(%s) should be disallowed\n",
+				 advices[i].name);
+	}
+
+	munmap(addr, pagesize);
+}
+
+static void test_munmap_splitting(void)
+{
+	char *addr1, *addr2;
+	int ret;
+
+	addr1 = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr1 == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* Unmap the first pages. */
+	ret = munmap(addr1, pagesize);
+	ksft_test_result(!ret, "munmap() splitting\n");
+
+	/* Remap the first page while the second page is still mapped. */
+	addr2 = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	ksft_test_result(addr2 != MAP_FAILED, "mmap() after splitting\n");
+
+	if (addr2 != MAP_FAILED)
+		munmap(addr2, pagesize);
+	if (!ret)
+		munmap(addr1 + pagesize, pagesize);
+	else
+		munmap(addr1, pagesize * 2);
+}
+
+static void test_mremap_fixed(void)
+{
+	char *addr, *new_addr, *ret;
+
+	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* Reserve a destination area. */
+	new_addr = mmap(0, pagesize * 2, PROT_READ, MAP_ANON | MAP_PRIVATE, -1, 0);
+	if (new_addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* mremap() over our destination. */
+	ret = mremap(addr, pagesize * 2, pagesize * 2,
+		     MREMAP_FIXED | MREMAP_MAYMOVE, new_addr);
+	ksft_test_result(ret == new_addr, "mremap(MREMAP_FIXED)\n");
+	if (ret != new_addr)
+		munmap(new_addr, pagesize * 2);
+	munmap(addr, pagesize * 2);
+}
+
+static void test_mremap_shrinking(void)
+{
+	char *addr, *ret;
+
+	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* Shrinking is expected to work. */
+	ret = mremap(addr, pagesize * 2, pagesize, 0);
+	ksft_test_result(ret == addr, "mremap() shrinking\n");
+	if (ret != addr)
+		munmap(addr, pagesize * 2);
+	else
+		munmap(addr, pagesize);
+}
+
+static void test_mremap_growing(void)
+{
+	char *addr, *ret;
+
+	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* Growing is not expected to work. */
+	ret = mremap(addr, pagesize, pagesize * 2, MREMAP_MAYMOVE);
+	ksft_test_result(ret == MAP_FAILED,
+			 "mremap() growing should be disallowed\n");
+	if (ret == MAP_FAILED)
+		munmap(addr, pagesize);
+	else
+		munmap(ret, pagesize * 2);
+}
+
+static void test_mprotect(void)
+{
+	char *addr, tmp;
+	int ret;
+
+	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* With PROT_NONE, read access must result in SIGSEGV. */
+	ret = mprotect(addr, pagesize, PROT_NONE);
+	ksft_test_result(!ret, "mprotect(PROT_NONE)\n");
+
+	ret = sigsetjmp(env, 1);
+	if (!ret) {
+		tmp = *addr;
+		asm volatile("" : "+r" (tmp));
+	}
+	ksft_test_result(ret == 1, "SIGSEGV expected\n");
+
+	/* With PROT_READ, read access must again succeed. */
+	ret = mprotect(addr, pagesize, PROT_READ);
+	ksft_test_result(!ret, "mprotect(PROT_READ)\n");
+
+	ret = sigsetjmp(env, 1);
+	if (!ret) {
+		tmp = *addr;
+		asm volatile("" : "+r" (tmp));
+	}
+	ksft_test_result(!ret, "SIGSEGV not expected\n");
+
+	munmap(addr, pagesize);
+}
+
+static void test_fork(void)
+{
+	char *addr, tmp;
+	int ret;
+
+	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
+	if (addr == MAP_FAILED)
+		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
+
+	/* fork() a child and test if the child can access the page. */
+	ret = fork();
+	if (ret < 0) {
+		ksft_test_result_fail("fork()\n");
+		goto out;
+	} else if (!ret) {
+		ret = sigsetjmp(env, 1);
+		if (!ret) {
+			tmp = *addr;
+			asm volatile("" : "+r" (tmp));
+		}
+		/* Return the result to the parent. */
+		exit(ret);
+	}
+	ksft_test_result_pass("fork()\n");
+
+	/* Wait for our child and obtain the result. */
+	wait(&ret);
+	if (WIFEXITED(ret))
+		ret = WEXITSTATUS(ret);
+	else
+		ret = -EINVAL;
+
+	ksft_test_result(!ret, "SIGSEGV in child not expected\n");
+out:
+	munmap(addr, pagesize);
+}
+
+int main(int argc, char **argv)
+{
+	int err;
+
+	ksft_print_header();
+	ksft_set_plan(19);
+
+	pagesize = getpagesize();
+	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
+	if (pagemap_fd < 0)
+		ksft_exit_fail_msg("opening pagemap failed\n");
+	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
+		ksft_exit_fail_msg("signal() failed: %s\n", strerror(errno));
+
+	sense_support();
+	test_madvise();
+	test_munmap_splitting();
+	test_mremap_fixed();
+	test_mremap_shrinking();
+	test_mremap_growing();
+	test_mprotect();
+	test_fork();
+
+	err = ksft_get_fail_cnt();
+	if (err)
+		ksft_exit_fail_msg("%d out of %d tests failed\n",
+				   err, ksft_test_num());
+	ksft_exit_pass();
+}
-- 
2.49.0

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[Dev Jain]

On 09/05/25 3:50 am, David Hildenbrand wrote:
> Let's test some basic functionality using /dev/mem. These tests will
> implicitly cover some PAT (Page Attribute Handling) handling on x86.
> 
> These tests will only run when /dev/mem access to the first two pages
> in physical address space is possible and allowed; otherwise, the tests
> are skipped.

Some generic comments:
1. I think you should also update .gitignore?
2. You can use ksft_exit_fail_perror() for wherever you want to print 
strerror(errno).

> 
> On current x86-64 with PAT inside a VM, all tests pass:
> 
> 	TAP version 13
> 	1..19
> 	ok 1 madvise(MADV_DONTNEED) should be disallowed
> 	ok 2 madvise(MADV_DONTNEED_LOCKED) should be disallowed
> 	ok 3 madvise(MADV_FREE) should be disallowed
> 	ok 4 madvise(MADV_WIPEONFORK) should be disallowed
> 	ok 5 madvise(MADV_COLD) should be disallowed
> 	ok 6 madvise(MADV_PAGEOUT) should be disallowed
> 	ok 7 madvise(MADV_POPULATE_READ) should be disallowed
> 	ok 8 madvise(MADV_POPULATE_WRITE) should be disallowed
> 	ok 9 munmap() splitting
> 	ok 10 mmap() after splitting
> 	ok 11 mremap(MREMAP_FIXED)
> 	ok 12 mremap() shrinking
> 	ok 13 mremap() growing should be disallowed
> 	ok 14 mprotect(PROT_NONE)
> 	ok 15 SIGSEGV expected
> 	ok 16 mprotect(PROT_READ)
> 	ok 17 SIGSEGV not expected
> 	ok 18 fork()
> 	ok 19 SIGSEGV in child not expected
> 	# Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> However, we are able to trigger:
> 
> [   27.888251] x86/PAT: pfnmap:1790 freeing invalid memtype [mem 0x00000000-0x00000fff]
> 
> There are probably more things worth testing in the future, such as
> MAP_PRIVATE handling. But this set of tests is sufficient to cover most of
> the things we will rework regarding PAT handling.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 
> On current mm-unstable, the MADV_POPULATE_READ test fails because
> mm-unstable contains a patch [1] that must be dropped.
> 
> [1] https://lore.kernel.org/all/20250507154105.763088-2-p.antoniou@partner.samsung.com/
> 
> ---
>   tools/testing/selftests/mm/Makefile |   1 +
>   tools/testing/selftests/mm/pfnmap.c | 278 ++++++++++++++++++++++++++++
>   2 files changed, 279 insertions(+)
>   create mode 100644 tools/testing/selftests/mm/pfnmap.c
> 
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index ad4d6043a60f0..ae6f994d3add7 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -84,6 +84,7 @@ TEST_GEN_FILES += mremap_test
>   TEST_GEN_FILES += mseal_test
>   TEST_GEN_FILES += on-fault-limit
>   TEST_GEN_FILES += pagemap_ioctl
> +TEST_GEN_FILES += pfnmap
>   TEST_GEN_FILES += thuge-gen
>   TEST_GEN_FILES += transhuge-stress
>   TEST_GEN_FILES += uffd-stress
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> new file mode 100644
> index 0000000000000..59be2f3221124
> --- /dev/null
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Basic VM_PFNMAP tests relying on mmap() of '/dev/mem'
> + *
> + * Copyright 2025, Red Hat, Inc.
> + *
> + * Author(s): David Hildenbrand <david@redhat.com>
> + */
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <setjmp.h>
> +#include <linux/mman.h>
> +#include <sys/mman.h>
> +#include <sys/wait.h>
> +
> +#include "../kselftest.h"
> +#include "vm_util.h"
> +
> +static size_t pagesize;
> +static int pagemap_fd;
> +static int dev_mem_fd;
> +static sigjmp_buf env;
> +
> +static void signal_handler(int sig)
> +{
> +	if (sig == SIGSEGV)
> +		siglongjmp(env, 1);
> +	siglongjmp(env, 2);
> +}
> +
> +static void sense_support(void)
> +{
> +	char *addr, tmp;
> +	int ret;
> +
> +	dev_mem_fd = open("/dev/mem", O_RDONLY);
> +	if (dev_mem_fd < 0)
> +		ksft_exit_skip("Cannot open '/dev/mem': %s\n", strerror(errno));
> +
> +	/* We'll require the first two pages throughout our tests ... */
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_skip("Cannot mmap '/dev/mem'");
> +
> +	/* ... and want to be able to read from them. */
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr + *(addr + pagesize);
> +		asm volatile("" : "+r" (tmp));
> +	}
> +	if (ret)
> +		ksft_exit_skip("Cannot read-access mmap'ed '/dev/mem'");
> +
> +	munmap(addr, pagesize * 2);
> +}
> +
> +static void test_madvise(void)
> +{
> +#define INIT_ADVICE(nr) { nr, #nr}
> +	const struct {
> +		int nr;
> +		const char *name;
> +	} advices[] = {
> +		INIT_ADVICE(MADV_DONTNEED),
> +		INIT_ADVICE(MADV_DONTNEED_LOCKED),
> +		INIT_ADVICE(MADV_FREE),
> +		INIT_ADVICE(MADV_WIPEONFORK),
> +		INIT_ADVICE(MADV_COLD),
> +		INIT_ADVICE(MADV_PAGEOUT),
> +		INIT_ADVICE(MADV_POPULATE_READ),
> +		INIT_ADVICE(MADV_POPULATE_WRITE),
> +	};
> +	char *addr;
> +	int ret, i;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* All these advices must be rejected. */
> +	for (i = 0; i < ARRAY_SIZE(advices); i++) {
> +		ret = madvise(addr, pagesize, advices[i].nr);
> +		ksft_test_result(ret && errno == EINVAL,
> +				 "madvise(%s) should be disallowed\n",
> +				 advices[i].name);
> +	}
> +
> +	munmap(addr, pagesize);
> +}
> +
> +static void test_munmap_splitting(void)
> +{
> +	char *addr1, *addr2;
> +	int ret;
> +
> +	addr1 = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr1 == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Unmap the first pages. */
> +	ret = munmap(addr1, pagesize);
> +	ksft_test_result(!ret, "munmap() splitting\n");
> +
> +	/* Remap the first page while the second page is still mapped. */
> +	addr2 = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	ksft_test_result(addr2 != MAP_FAILED, "mmap() after splitting\n");
> +
> +	if (addr2 != MAP_FAILED)
> +		munmap(addr2, pagesize);
> +	if (!ret)
> +		munmap(addr1 + pagesize, pagesize);
> +	else
> +		munmap(addr1, pagesize * 2);
> +}
> +
> +static void test_mremap_fixed(void)
> +{
> +	char *addr, *new_addr, *ret;
> +
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Reserve a destination area. */
> +	new_addr = mmap(0, pagesize * 2, PROT_READ, MAP_ANON | MAP_PRIVATE, -1, 0);
> +	if (new_addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* mremap() over our destination. */
> +	ret = mremap(addr, pagesize * 2, pagesize * 2,
> +		     MREMAP_FIXED | MREMAP_MAYMOVE, new_addr);
> +	ksft_test_result(ret == new_addr, "mremap(MREMAP_FIXED)\n");
> +	if (ret != new_addr)
> +		munmap(new_addr, pagesize * 2);
> +	munmap(addr, pagesize * 2);
> +}
> +
> +static void test_mremap_shrinking(void)
> +{
> +	char *addr, *ret;
> +
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Shrinking is expected to work. */
> +	ret = mremap(addr, pagesize * 2, pagesize, 0);
> +	ksft_test_result(ret == addr, "mremap() shrinking\n");
> +	if (ret != addr)
> +		munmap(addr, pagesize * 2);
> +	else
> +		munmap(addr, pagesize);
> +}
> +
> +static void test_mremap_growing(void)
> +{
> +	char *addr, *ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Growing is not expected to work. */
> +	ret = mremap(addr, pagesize, pagesize * 2, MREMAP_MAYMOVE);
> +	ksft_test_result(ret == MAP_FAILED,
> +			 "mremap() growing should be disallowed\n");
> +	if (ret == MAP_FAILED)
> +		munmap(addr, pagesize);
> +	else
> +		munmap(ret, pagesize * 2);
> +}
> +
> +static void test_mprotect(void)
> +{
> +	char *addr, tmp;
> +	int ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* With PROT_NONE, read access must result in SIGSEGV. */
> +	ret = mprotect(addr, pagesize, PROT_NONE);
> +	ksft_test_result(!ret, "mprotect(PROT_NONE)\n");
> +
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr;
> +		asm volatile("" : "+r" (tmp));
> +	}
> +	ksft_test_result(ret == 1, "SIGSEGV expected\n");
> +
> +	/* With PROT_READ, read access must again succeed. */
> +	ret = mprotect(addr, pagesize, PROT_READ);
> +	ksft_test_result(!ret, "mprotect(PROT_READ)\n");
> +
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr;
> +		asm volatile("" : "+r" (tmp));
> +	}
> +	ksft_test_result(!ret, "SIGSEGV not expected\n");
> +
> +	munmap(addr, pagesize);
> +}
> +
> +static void test_fork(void)
> +{
> +	char *addr, tmp;
> +	int ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* fork() a child and test if the child can access the page. */
> +	ret = fork();
> +	if (ret < 0) {
> +		ksft_test_result_fail("fork()\n");
> +		goto out;
> +	} else if (!ret) {
> +		ret = sigsetjmp(env, 1);
> +		if (!ret) {
> +			tmp = *addr;
> +			asm volatile("" : "+r" (tmp));
> +		}
> +		/* Return the result to the parent. */
> +		exit(ret);
> +	}
> +	ksft_test_result_pass("fork()\n");
> +
> +	/* Wait for our child and obtain the result. */
> +	wait(&ret);
> +	if (WIFEXITED(ret))
> +		ret = WEXITSTATUS(ret);
> +	else
> +		ret = -EINVAL;
> +
> +	ksft_test_result(!ret, "SIGSEGV in child not expected\n");
> +out:
> +	munmap(addr, pagesize);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int err;
> +
> +	ksft_print_header();
> +	ksft_set_plan(19);
> +
> +	pagesize = getpagesize();
> +	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
> +	if (pagemap_fd < 0)
> +		ksft_exit_fail_msg("opening pagemap failed\n");
> +	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
> +		ksft_exit_fail_msg("signal() failed: %s\n", strerror(errno));
> +
> +	sense_support();
> +	test_madvise();
> +	test_munmap_splitting();
> +	test_mremap_fixed();
> +	test_mremap_shrinking();
> +	test_mremap_growing();
> +	test_mprotect();
> +	test_fork();
> +
> +	err = ksft_get_fail_cnt();
> +	if (err)
> +		ksft_exit_fail_msg("%d out of %d tests failed\n",
> +				   err, ksft_test_num());
> +	ksft_exit_pass();
> +}

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[David Hildenbrand]

On 09.05.25 07:35, Dev Jain wrote:
> 
> 
> On 09/05/25 3:50 am, David Hildenbrand wrote:
>> Let's test some basic functionality using /dev/mem. These tests will
>> implicitly cover some PAT (Page Attribute Handling) handling on x86.
>>
>> These tests will only run when /dev/mem access to the first two pages
>> in physical address space is possible and allowed; otherwise, the tests
>> are skipped.
> 

Hi Dev,

> Some generic comments:
> 1. I think you should also update .gitignore?

I wonder if we could add a checkpatch warning for that, it keeps on 
happening :)

> 2. You can use ksft_exit_fail_perror() for wherever you want to print
> strerror(errno).


Makes sense, thanks!


-- 
Cheers,

David / dhildenb

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[Lorenzo Stoakes]

On Fri, May 09, 2025 at 12:20:41AM +0200, David Hildenbrand wrote:
> Let's test some basic functionality using /dev/mem. These tests will
> implicitly cover some PAT (Page Attribute Handling) handling on x86.

Ah this is really nice thanks for this!

/dev/mem is a good choice as a straightforward way to get VM_PFNMAP
mappings also.

>
> These tests will only run when /dev/mem access to the first two pages
> in physical address space is possible and allowed; otherwise, the tests
> are skipped.
>
> On current x86-64 with PAT inside a VM, all tests pass:
>
> 	TAP version 13
> 	1..19
> 	ok 1 madvise(MADV_DONTNEED) should be disallowed
> 	ok 2 madvise(MADV_DONTNEED_LOCKED) should be disallowed
> 	ok 3 madvise(MADV_FREE) should be disallowed
> 	ok 4 madvise(MADV_WIPEONFORK) should be disallowed
> 	ok 5 madvise(MADV_COLD) should be disallowed
> 	ok 6 madvise(MADV_PAGEOUT) should be disallowed
> 	ok 7 madvise(MADV_POPULATE_READ) should be disallowed
> 	ok 8 madvise(MADV_POPULATE_WRITE) should be disallowed
> 	ok 9 munmap() splitting
> 	ok 10 mmap() after splitting
> 	ok 11 mremap(MREMAP_FIXED)
> 	ok 12 mremap() shrinking
> 	ok 13 mremap() growing should be disallowed

One could argue these should be in mremap tests, but the mremap tests are a
bit of a mess and I think better here.

> 	ok 14 mprotect(PROT_NONE)
> 	ok 15 SIGSEGV expected
> 	ok 16 mprotect(PROT_READ)
> 	ok 17 SIGSEGV not expected
> 	ok 18 fork()
> 	ok 19 SIGSEGV in child not expected
> 	# Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0

It'd be good to assert that merging doesn't work for VM_PFNMAP, though hm
one could argue that's not hugely useful as it's trivially implemented.

But I guess anything like that should live in merge.c.

>
> However, we are able to trigger:
>
> [   27.888251] x86/PAT: pfnmap:1790 freeing invalid memtype [mem 0x00000000-0x00000fff]
>
> There are probably more things worth testing in the future, such as
> MAP_PRIVATE handling. But this set of tests is sufficient to cover most of
> the things we will rework regarding PAT handling.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> On current mm-unstable, the MADV_POPULATE_READ test fails because
> mm-unstable contains a patch [1] that must be dropped.
>
> [1] https://lore.kernel.org/all/20250507154105.763088-2-p.antoniou@partner.samsung.com/
>
> ---
>  tools/testing/selftests/mm/Makefile |   1 +
>  tools/testing/selftests/mm/pfnmap.c | 278 ++++++++++++++++++++++++++++

As Dev points out you should update .gitignore, but you should also update
run_vmtests.sh so this gets run with everything else!

>  2 files changed, 279 insertions(+)
>  create mode 100644 tools/testing/selftests/mm/pfnmap.c
>
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index ad4d6043a60f0..ae6f994d3add7 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -84,6 +84,7 @@ TEST_GEN_FILES += mremap_test
>  TEST_GEN_FILES += mseal_test
>  TEST_GEN_FILES += on-fault-limit
>  TEST_GEN_FILES += pagemap_ioctl
> +TEST_GEN_FILES += pfnmap
>  TEST_GEN_FILES += thuge-gen
>  TEST_GEN_FILES += transhuge-stress
>  TEST_GEN_FILES += uffd-stress
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> new file mode 100644
> index 0000000000000..59be2f3221124
> --- /dev/null
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Basic VM_PFNMAP tests relying on mmap() of '/dev/mem'
> + *
> + * Copyright 2025, Red Hat, Inc.
> + *
> + * Author(s): David Hildenbrand <david@redhat.com>
> + */
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <setjmp.h>
> +#include <linux/mman.h>
> +#include <sys/mman.h>
> +#include <sys/wait.h>
> +
> +#include "../kselftest.h"
> +#include "vm_util.h"
> +
> +static size_t pagesize;
> +static int pagemap_fd;
> +static int dev_mem_fd;
> +static sigjmp_buf env;
> +
> +static void signal_handler(int sig)
> +{
> +	if (sig == SIGSEGV)
> +		siglongjmp(env, 1);
> +	siglongjmp(env, 2);
> +}

Hm, wouldn't it be better to only catch these only if you specifically
meant to catch a signal?

You can see what I did in guard-regions.c for an example (sorry, I'm sure
you know exactly how the thing works, just I mean for an easy reminder :P)

> +
> +static void sense_support(void)
> +{

See below comment about the kselftest_harness, but with that you can
literally declare fixture setups/teardowns very nicely :) You can also
mmap() these 2 pages and munmap() them afterwards straightforwardly.

> +	char *addr, tmp;
> +	int ret;
> +
> +	dev_mem_fd = open("/dev/mem", O_RDONLY);
> +	if (dev_mem_fd < 0)
> +		ksft_exit_skip("Cannot open '/dev/mem': %s\n", strerror(errno));

Hm skip, or failure? Skip implies it's expected right? I suppose it's
possible a system might be setup without this...

> +
> +	/* We'll require the first two pages throughout our tests ... */
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_skip("Cannot mmap '/dev/mem'");
> +
> +	/* ... and want to be able to read from them. */
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr + *(addr + pagesize);
> +		asm volatile("" : "+r" (tmp));

Is this not pretty much equivalent to a volatile read where you're forcing
the compiler to not optimise this unused thing away? In guard-regions I set:

#define FORCE_READ(x) (*(volatile typeof(x) *)x)

For this purpose, which would make this:

FORCE_READ(addr);
FORCE_READ(&addr[pagesize]);

> +	}
> +	if (ret)
> +		ksft_exit_skip("Cannot read-access mmap'ed '/dev/mem'");

Why are we returning 1 or 2 if we don't differentiate it here?

> +
> +	munmap(addr, pagesize * 2);
> +}
> +
> +static void test_madvise(void)
> +{
> +#define INIT_ADVICE(nr) { nr, #nr}
> +	const struct {
> +		int nr;
> +		const char *name;
> +	} advices[] = {
> +		INIT_ADVICE(MADV_DONTNEED),
> +		INIT_ADVICE(MADV_DONTNEED_LOCKED),
> +		INIT_ADVICE(MADV_FREE),
> +		INIT_ADVICE(MADV_WIPEONFORK),
> +		INIT_ADVICE(MADV_COLD),
> +		INIT_ADVICE(MADV_PAGEOUT),
> +		INIT_ADVICE(MADV_POPULATE_READ),
> +		INIT_ADVICE(MADV_POPULATE_WRITE),
> +	};
> +	char *addr;
> +	int ret, i;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);

Nit (same for all mmap() calls) shouldn't this first parameter be NULL, by
convention? I mean not a big deal obviously :)

> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* All these advices must be rejected. */
> +	for (i = 0; i < ARRAY_SIZE(advices); i++) {
> +		ret = madvise(addr, pagesize, advices[i].nr);
> +		ksft_test_result(ret && errno == EINVAL,
> +				 "madvise(%s) should be disallowed\n",
> +				 advices[i].name);
> +	}
> +
> +	munmap(addr, pagesize);
> +}
> +
> +static void test_munmap_splitting(void)
> +{
> +	char *addr1, *addr2;
> +	int ret;
> +
> +	addr1 = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr1 == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Unmap the first pages. */

NIT: pages -> page.

> +	ret = munmap(addr1, pagesize);
> +	ksft_test_result(!ret, "munmap() splitting\n");
> +
> +	/* Remap the first page while the second page is still mapped. */
> +	addr2 = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	ksft_test_result(addr2 != MAP_FAILED, "mmap() after splitting\n");

Hm not sure what the assertion is here per se, that we can munmap() partial
bits of the VMA? It'd be pretty weird if we couldn't though?

If it's that we don't get a merge when we remap, we're not really checking
that, but you actually can, as I added an API to vm_util for this using
PROCMAP_QUERY (very handy tool actually - binary version of /proc/smaps).

You can then use that to determine if the VMAs are separate, see merge.c
for examples, it's actually really quite easy to use, e.g.:

	ASSERT_TRUE(find_vma_procmap(procmap, ptr));
	ASSERT_EQ(procmap->query.vma_start, (unsigned long)ptr);
	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 10 * page_size);

> +
> +	if (addr2 != MAP_FAILED)
> +		munmap(addr2, pagesize);
> +	if (!ret)
> +		munmap(addr1 + pagesize, pagesize);
> +	else
> +		munmap(addr1, pagesize * 2);

There's no need for this dance, you can just munmap() away, it tolerates
gaps and multiple VMAs.

> +}
> +
> +static void test_mremap_fixed(void)
> +{
> +	char *addr, *new_addr, *ret;
> +
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Reserve a destination area. */
> +	new_addr = mmap(0, pagesize * 2, PROT_READ, MAP_ANON | MAP_PRIVATE, -1, 0);
> +	if (new_addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* mremap() over our destination. */
> +	ret = mremap(addr, pagesize * 2, pagesize * 2,
> +		     MREMAP_FIXED | MREMAP_MAYMOVE, new_addr);
> +	ksft_test_result(ret == new_addr, "mremap(MREMAP_FIXED)\n");
> +	if (ret != new_addr)
> +		munmap(new_addr, pagesize * 2);

This could only be an error code, and this will fail right?

MREMAP_FIXED is 'do or die' at the new address, not hinting. If there's
anything already mapped there it goes a bye bye.

So again, we could just have a standard munmap(), and this lends itself
well to a FIXTURE_SETUP()/FIXTURE_TEARDOWN() :P

> +	munmap(addr, pagesize * 2);
> +}
> +
> +static void test_mremap_shrinking(void)
> +{
> +	char *addr, *ret;
> +
> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Shrinking is expected to work. */
> +	ret = mremap(addr, pagesize * 2, pagesize, 0);
> +	ksft_test_result(ret == addr, "mremap() shrinking\n");
> +	if (ret != addr)
> +		munmap(addr, pagesize * 2);
> +	else
> +		munmap(addr, pagesize);

I think we're safe to just munmap() as usual here :) (it's nitty but I'm
trying to make the case for teardown again of course :P)

> +}
> +
> +static void test_mremap_growing(void)
> +{
> +	char *addr, *ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* Growing is not expected to work. */

God imagine if we did allow it... what hell would it be to figure out how
to do this correctly in all cases :P

> +	ret = mremap(addr, pagesize, pagesize * 2, MREMAP_MAYMOVE);
> +	ksft_test_result(ret == MAP_FAILED,
> +			 "mremap() growing should be disallowed\n");
> +	if (ret == MAP_FAILED)
> +		munmap(addr, pagesize);
> +	else
> +		munmap(ret, pagesize * 2);

This is a bit cautious, for a world where we do lose our minds and allow
this? :)

> +}
> +
> +static void test_mprotect(void)
> +{
> +	char *addr, tmp;
> +	int ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* With PROT_NONE, read access must result in SIGSEGV. */
> +	ret = mprotect(addr, pagesize, PROT_NONE);
> +	ksft_test_result(!ret, "mprotect(PROT_NONE)\n");
> +
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr;
> +		asm volatile("" : "+r" (tmp));
> +	}

This code is duplicated, we definitely want to abstract it.

> +	ksft_test_result(ret == 1, "SIGSEGV expected\n");

Hmm, what exactly are we testing here though? I mean PROT_NONE will be a
failed access for _any_ kind of memory? Is this really worthwhile? Maybe
better to mprotect() as PROT_NONE to start then mprotect() to PROT_READ.

But I'm not sure what that really tests? Is it a PAT-specific thing? It
seems if this is broken then the mapping code is more generally broken
beyond just VM_PFNMAP mappings right?

> +
> +	/* With PROT_READ, read access must again succeed. */
> +	ret = mprotect(addr, pagesize, PROT_READ);
> +	ksft_test_result(!ret, "mprotect(PROT_READ)\n");
> +
> +	ret = sigsetjmp(env, 1);
> +	if (!ret) {
> +		tmp = *addr;
> +		asm volatile("" : "+r" (tmp));
> +	}

And also duplicated here.

> +	ksft_test_result(!ret, "SIGSEGV not expected\n");
> +
> +	munmap(addr, pagesize);
> +}
> +
> +static void test_fork(void)
> +{
> +	char *addr, tmp;
> +	int ret;
> +
> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> +	if (addr == MAP_FAILED)
> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
> +
> +	/* fork() a child and test if the child can access the page. */
> +	ret = fork();
> +	if (ret < 0) {
> +		ksft_test_result_fail("fork()\n");
> +		goto out;
> +	} else if (!ret) {
> +		ret = sigsetjmp(env, 1);
> +		if (!ret) {
> +			tmp = *addr;
> +			asm volatile("" : "+r" (tmp));
> +		}

Same comment as above re: duplication.

> +		/* Return the result to the parent. */
> +		exit(ret);
> +	}
> +	ksft_test_result_pass("fork()\n");
> +
> +	/* Wait for our child and obtain the result. */
> +	wait(&ret);
> +	if (WIFEXITED(ret))
> +		ret = WEXITSTATUS(ret);
> +	else
> +		ret = -EINVAL;
> +
> +	ksft_test_result(!ret, "SIGSEGV in child not expected\n");
> +out:
> +	munmap(addr, pagesize);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int err;
> +
> +	ksft_print_header();
> +	ksft_set_plan(19);

I know it's kind of nitpicky, but I really hate this sort of magic number
and so on. You don't actually need any of this, the kselftest_harness.h is
_really_ powerful, and makes for much much more readable and standardised
test code.

You can look at guard-regions.c in the test code (though there's some
complexity there because I use 'variants') or the merge.c test code
(simpler) for straight-forward examples.

I won't block this change on this however, I don't want to be a pain and
you're adding very important tests here, but it'd be really nice if you did
use that :>)

> +
> +	pagesize = getpagesize();
> +	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
> +	if (pagemap_fd < 0)
> +		ksft_exit_fail_msg("opening pagemap failed\n");
> +	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
> +		ksft_exit_fail_msg("signal() failed: %s\n", strerror(errno));
> +
> +	sense_support();
> +	test_madvise();
> +	test_munmap_splitting();
> +	test_mremap_fixed();
> +	test_mremap_shrinking();
> +	test_mremap_growing();
> +	test_mprotect();
> +	test_fork();
> +
> +	err = ksft_get_fail_cnt();
> +	if (err)
> +		ksft_exit_fail_msg("%d out of %d tests failed\n",
> +				   err, ksft_test_num());
> +	ksft_exit_pass();
> +}
> --
> 2.49.0
>

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[David Hildenbrand]

On 09.05.25 11:49, Lorenzo Stoakes wrote:
> On Fri, May 09, 2025 at 12:20:41AM +0200, David Hildenbrand wrote:
>> Let's test some basic functionality using /dev/mem. These tests will
>> implicitly cover some PAT (Page Attribute Handling) handling on x86.
> 
> Ah this is really nice thanks for this!

Thanks for your review!

> 
>> 	ok 14 mprotect(PROT_NONE)
>> 	ok 15 SIGSEGV expected
>> 	ok 16 mprotect(PROT_READ)
>> 	ok 17 SIGSEGV not expected
>> 	ok 18 fork()
>> 	ok 19 SIGSEGV in child not expected
>> 	# Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> It'd be good to assert that merging doesn't work for VM_PFNMAP, though hm
> one could argue that's not hugely useful as it's trivially implemented.
> 
> But I guess anything like that should live in merge.c.

I assume we'd need is_range_mapped() from mremap_tests.c.

Something for another day :)

[...]

>> +static void signal_handler(int sig)
>> +{
>> +	if (sig == SIGSEGV)
>> +		siglongjmp(env, 1);
>> +	siglongjmp(env, 2);
>> +}
> 
> Hm, wouldn't it be better to only catch these only if you specifically
> meant to catch a signal?

I had that, but got tired about the repeated register + unregister, 
after all I really don't want to spend a lot more time on this.

> You can see what I did in guard-regions.c for an example (sorry, I'm sure
> you know exactly how the thing works, just I mean for an easy reminder :P)
> 

Again, time is the limit. But let me see if I can get something done in 
a reasonable timeframe.

>> +
>> +static void sense_support(void)
>> +{
> 
> See below comment about the kselftest_harness, but with that you can
> literally declare fixture setups/teardowns very nicely :) You can also
> mmap() these 2 pages and munmap() them afterwards straightforwardly.
> 
>> +	char *addr, tmp;
>> +	int ret;
>> +
>> +	dev_mem_fd = open("/dev/mem", O_RDONLY);
>> +	if (dev_mem_fd < 0)
>> +		ksft_exit_skip("Cannot open '/dev/mem': %s\n", strerror(errno));
> 
> Hm skip, or failure? Skip implies it's expected right? I suppose it's
> possible a system might be setup without this...

Try as non-root or on a lockdowned system :)

> 
>> +
>> +	/* We'll require the first two pages throughout our tests ... */
>> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_skip("Cannot mmap '/dev/mem'");
>> +
>> +	/* ... and want to be able to read from them. */
>> +	ret = sigsetjmp(env, 1);
>> +	if (!ret) {
>> +		tmp = *addr + *(addr + pagesize);
>> +		asm volatile("" : "+r" (tmp));
> 
> Is this not pretty much equivalent to a volatile read where you're forcing
> the compiler to not optimise this unused thing away? In guard-regions I set:
> 
> #define FORCE_READ(x) (*(volatile typeof(x) *)x)
> 
> For this purpose, which would make this:
> 
> FORCE_READ(addr);
> FORCE_READ(&addr[pagesize]);

Hmmm, a compiler might be allowed to optimize out a volatile read.

> 
>> +	}
>> +	if (ret)
>> +		ksft_exit_skip("Cannot read-access mmap'ed '/dev/mem'");
> 
> Why are we returning 1 or 2 if we don't differentiate it here?

Copy-and-paste. As we are not registering for SIGBUS, we can just return 1.

> 
>> +
>> +	munmap(addr, pagesize * 2);
>> +}
>> +
>> +static void test_madvise(void)
>> +{
>> +#define INIT_ADVICE(nr) { nr, #nr}
>> +	const struct {
>> +		int nr;
>> +		const char *name;
>> +	} advices[] = {
>> +		INIT_ADVICE(MADV_DONTNEED),
>> +		INIT_ADVICE(MADV_DONTNEED_LOCKED),
>> +		INIT_ADVICE(MADV_FREE),
>> +		INIT_ADVICE(MADV_WIPEONFORK),
>> +		INIT_ADVICE(MADV_COLD),
>> +		INIT_ADVICE(MADV_PAGEOUT),
>> +		INIT_ADVICE(MADV_POPULATE_READ),
>> +		INIT_ADVICE(MADV_POPULATE_WRITE),
>> +	};
>> +	char *addr;
>> +	int ret, i;
>> +
>> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
> 
> Nit (same for all mmap() calls) shouldn't this first parameter be NULL, by
> convention? I mean not a big deal obviously :)

Yes.

> 
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* All these advices must be rejected. */
>> +	for (i = 0; i < ARRAY_SIZE(advices); i++) {
>> +		ret = madvise(addr, pagesize, advices[i].nr);
>> +		ksft_test_result(ret && errno == EINVAL,
>> +				 "madvise(%s) should be disallowed\n",
>> +				 advices[i].name);
>> +	}
>> +
>> +	munmap(addr, pagesize);
>> +}
>> +
>> +static void test_munmap_splitting(void)
>> +{
>> +	char *addr1, *addr2;
>> +	int ret;
>> +
>> +	addr1 = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr1 == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* Unmap the first pages. */
> 
> NIT: pages -> page.

Ack.

> 
>> +	ret = munmap(addr1, pagesize);
>> +	ksft_test_result(!ret, "munmap() splitting\n");
>> +
>> +	/* Remap the first page while the second page is still mapped. */
>> +	addr2 = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	ksft_test_result(addr2 != MAP_FAILED, "mmap() after splitting\n");
> 
> Hm not sure what the assertion is here per se, that we can munmap() partial
> bits of the VMA? It'd be pretty weird if we couldn't though?
 > > If it's that we don't get a merge when we remap, we're not really 
checking
> that, but you actually can, as I added an API to vm_util for this using
> PROCMAP_QUERY (very handy tool actually - binary version of /proc/smaps).

I don't care about merging tests (I'll leave that to you :P ).

This is a PAT test for upcoming changes where partial unmap can leave 
the original region reserved. Making sure that re-mapping with the 
pending reservation still works.

>> +
>> +	if (addr2 != MAP_FAILED)
>> +		munmap(addr2, pagesize);
>> +	if (!ret)
>> +		munmap(addr1 + pagesize, pagesize);
>> +	else
>> +		munmap(addr1, pagesize * 2);
> 
> There's no need for this dance, you can just munmap() away, it tolerates
> gaps and multiple VMAs.

Yeah, I know. I was not sure if the ksft_test_result() in between might 
allocate memory and consume that area.

> 
>> +}
>> +
>> +static void test_mremap_fixed(void)
>> +{
>> +	char *addr, *new_addr, *ret;
>> +
>> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* Reserve a destination area. */
>> +	new_addr = mmap(0, pagesize * 2, PROT_READ, MAP_ANON | MAP_PRIVATE, -1, 0);
>> +	if (new_addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* mremap() over our destination. */
>> +	ret = mremap(addr, pagesize * 2, pagesize * 2,
>> +		     MREMAP_FIXED | MREMAP_MAYMOVE, new_addr);
>> +	ksft_test_result(ret == new_addr, "mremap(MREMAP_FIXED)\n");
>> +	if (ret != new_addr)
>> +		munmap(new_addr, pagesize * 2);
> 
> This could only be an error code, and this will fail right?
> 
> MREMAP_FIXED is 'do or die' at the new address, not hinting. If there's
> anything already mapped there it goes a bye bye.
> 
> So again, we could just have a standard munmap(), and this lends itself
> well to a FIXTURE_SETUP()/FIXTURE_TEARDOWN() :P

I'm afraid I cannot spend much more time on these tests :P But let me 
try for a couple of minutes.

> 
>> +	munmap(addr, pagesize * 2);
>> +}
>> +
>> +static void test_mremap_shrinking(void)
>> +{
>> +	char *addr, *ret;
>> +
>> +	addr = mmap(0, pagesize * 2, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* Shrinking is expected to work. */
>> +	ret = mremap(addr, pagesize * 2, pagesize, 0);
>> +	ksft_test_result(ret == addr, "mremap() shrinking\n");
>> +	if (ret != addr)
>> +		munmap(addr, pagesize * 2);
>> +	else
>> +		munmap(addr, pagesize);
> 
> I think we're safe to just munmap() as usual here :) (it's nitty but I'm
> trying to make the case for teardown again of course :P)

Same reasoning as above regarding ksft_test_result().

> 
>> +}
>> +
>> +static void test_mremap_growing(void)
>> +{
>> +	char *addr, *ret;
>> +
>> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* Growing is not expected to work. */
> 
> God imagine if we did allow it... what hell would it be to figure out how
> to do this correctly in all cases :P

:)

> 
>> +	ret = mremap(addr, pagesize, pagesize * 2, MREMAP_MAYMOVE);
>> +	ksft_test_result(ret == MAP_FAILED,
>> +			 "mremap() growing should be disallowed\n");
>> +	if (ret == MAP_FAILED)
>> +		munmap(addr, pagesize);
>> +	else
>> +		munmap(ret, pagesize * 2);
> 
> This is a bit cautious, for a world where we do lose our minds and allow
> this? :)

Yeah, went back and forth with this error cleanup shit.

> 
>> +}
>> +
>> +static void test_mprotect(void)
>> +{
>> +	char *addr, tmp;
>> +	int ret;
>> +
>> +	addr = mmap(0, pagesize, PROT_READ, MAP_SHARED, dev_mem_fd, 0);
>> +	if (addr == MAP_FAILED)
>> +		ksft_exit_fail_msg("mmap() failed: %s\n", strerror(errno));
>> +
>> +	/* With PROT_NONE, read access must result in SIGSEGV. */
>> +	ret = mprotect(addr, pagesize, PROT_NONE);
>> +	ksft_test_result(!ret, "mprotect(PROT_NONE)\n");
>> +
>> +	ret = sigsetjmp(env, 1);
>> +	if (!ret) {
>> +		tmp = *addr;
>> +		asm volatile("" : "+r" (tmp));
>> +	}
> 
> This code is duplicated, we definitely want to abstract it.

Probably yes.

> 
>> +	ksft_test_result(ret == 1, "SIGSEGV expected\n");
> 
> Hmm, what exactly are we testing here though? I mean PROT_NONE will be a
> failed access for _any_ kind of memory? Is this really worthwhile? Maybe
> better to mprotect() as PROT_NONE to start then mprotect() to PROT_READ.
 > > But I'm not sure what that really tests? Is it a PAT-specific thing? It
> seems if this is broken then the mapping code is more generally broken
> beyond just VM_PFNMAP mappings right?

Rationale was to test the !vm_normal_folio() code paths that are not 
covered by "ordinary" mprotect (except the shared zeropage). But there 
should indeed only be such a check on the prot_numa code path, so I can 
just drop this test.

[...]

>> +int main(int argc, char **argv)
>> +{
>> +	int err;
>> +
>> +	ksft_print_header();
>> +	ksft_set_plan(19);
> 
> I know it's kind of nitpicky, but I really hate this sort of magic number
> and so on. You don't actually need any of this, the kselftest_harness.h is
> _really_ powerful, and makes for much much more readable and standardised
> test code.
> 
> You can look at guard-regions.c in the test code (though there's some
> complexity there because I use 'variants') or the merge.c test code
> (simpler) for straight-forward examples.
> 
> I won't block this change on this however, I don't want to be a pain and
> you're adding very important tests here, but it'd be really nice if you did
> use that :>)

Yeah, let me explore that real quick, thanks!

-- 
Cheers,

David / dhildenb

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[David Hildenbrand]

>> Is this not pretty much equivalent to a volatile read where you're forcing
>> the compiler to not optimise this unused thing away? In guard-regions I set:
>>
>> #define FORCE_READ(x) (*(volatile typeof(x) *)x)
>>
>> For this purpose, which would make this:
>>
>> FORCE_READ(addr);
>> FORCE_READ(&addr[pagesize]);
> 
> Hmmm, a compiler might be allowed to optimize out a volatile read.

Looking into this, the compiler should not be allowed to do that. So 
FORCE_READ() should work!

-- 
Cheers,

David / dhildenb

Re: [PATCH v1] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[Lorenzo Stoakes]

On Fri, May 09, 2025 at 12:43:49PM +0200, David Hildenbrand wrote:
> > > Is this not pretty much equivalent to a volatile read where you're forcing
> > > the compiler to not optimise this unused thing away? In guard-regions I set:
> > >
> > > #define FORCE_READ(x) (*(volatile typeof(x) *)x)
> > >
> > > For this purpose, which would make this:
> > >
> > > FORCE_READ(addr);
> > > FORCE_READ(&addr[pagesize]);
> >
> > Hmmm, a compiler might be allowed to optimize out a volatile read.
>
> Looking into this, the compiler should not be allowed to do that. So
> FORCE_READ() should work!

Yeah, was going to say I thought the compiler was explicitly forbidden from
doing this so is a rare case of 'volatile considered harmful' not being
quite so harmful :P

>
> --
> Cheers,
>
> David / dhildenb
>