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.samsu...
--- 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(); +}
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.samsu...
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();
+}
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:
- I think you should also update .gitignore?
I wonder if we could add a checkpatch warning for that, it keeps on happening :)
- You can use ksft_exit_fail_perror() for wherever you want to print
strerror(errno).
Makes sense, thanks!
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.samsu...
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
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!
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!
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
linux-kselftest-mirror@lists.linaro.org
-
David Hildenbrand -
Dev Jain -
Lorenzo Stoakes