Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28.05.25 20:23, Aishwarya wrote: Hi David, I applied the patch on next-20250515 and ran the test. The issue is no longer observed, and the test completed successfully. I can confirm that the patch resolves the problem. Tested-by: Aishwarya TCV Thanks a bunch for the fast re-test! -- Cheers, David / dhildenb
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
Hi David, I applied the patch on next-20250515 and ran the test. The issue is no longer observed, and the test completed successfully. I can confirm that the patch resolves the problem. Tested-by: Aishwarya TCV Thanks, Aishwarya
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28/05/2025 13:40, David Hildenbrand wrote: > On 28.05.25 12:53, Ryan Roberts wrote: >> On 28/05/2025 11:48, David Hildenbrand wrote: >>> On 28.05.25 12:44, David Hildenbrand wrote: On 28.05.25 12:34, Ryan Roberts wrote: > Hi David, > > > On 09/05/2025 16:30, 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. > > We are seeing really horrible RAS errors with this test when run on arm64 > tx2 > machine. Based solely on reviewing the code, I think the problem is that > tx2 > doesn't have anything at phys address 0, so test_read_access() is trying > to > put > trasactions out to a bad address on the bus. > > tx2 /proc/iomem: > > $ sudo cat /proc/iomem > 3000-37ff : PCI ECAM > 3800-3fff : PCI ECAM > 4000-5fff : PCI Bus :00 > ... > > Whereas my x86 box has some reserved memory: > > $ sudo cat /proc/iomem > -0fff : Reserved > 1000-0003dfff : System RAM > ... > A quick fix would be to make this test specific to x86 (the only one I tested on). We should always have the lower two pages IIRC (BIOS stuff etc). >> >> I'm not sure how far along this patch is? I'm guessing mm-stable? Perhaps you >> can do the quick fix, then I'd be happy to make this more robust for arm64 >> later? > > Can you give the following a quick test on that machine? Then, I can send it > as a > proper patch later. The machine in question is part of our CI infra, so not easy for me to run an ad-hoc test. I've asked Aishwarya if it's possible to queue up a CI job with the patch, but that will involve running the whole test run I think, so probably will take a couple of days to turn around. FWIW, the change looks good to me: Reviewed-by: Ryan Roberts > > > From 40fea063f2fcf1474fb47cb9aebdb04fd825032b Mon Sep 17 00:00:00 2001 > From: David Hildenbrand > Date: Wed, 28 May 2025 14:35:23 +0200 > Subject: [PATCH] selftests/mm: two fixes for the pfnmap test > > When unregistering the signal handler, we have to pass SIG_DFL, and > blindly reading from PFN 0 and PFN 1 seems to be problematic on !x86 > systems. In particularly, on arm64 tx2 machines where noting resides > at these physical memory locations, we can generate RAS errors. > > Let's fix it by scanning /proc/iomem for actual "System RAM". > > Reported-by: Ryan Roberts > Closes: https://lore.kernel.org/all/232960c2-81db-47ca- > [email protected]/T/#u > Fixes: 2616b370323a ("selftests/mm: add simple VM_PFNMAP tests based on > mmap'ing /dev/mem") > Signed-off-by: David Hildenbrand > --- > tools/testing/selftests/mm/pfnmap.c | 61 +++-- > 1 file changed, 57 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/ > pfnmap.c > index 8a9d19b6020c7..4943927a7d1ea 100644 > --- a/tools/testing/selftests/mm/pfnmap.c > +++ b/tools/testing/selftests/mm/pfnmap.c > @@ -12,6 +12,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -43,14 +45,62 @@ static int test_read_access(char *addr, size_t size, > size_t > pagesize) > /* Force a read that the compiler cannot optimize out. */ > *((volatile char *)(addr + offs)); > } > - if (signal(SIGSEGV, signal_handler) == SIG_ERR) > + if (signal(SIGSEGV, SIG_DFL) == SIG_ERR) > return -EINVAL; > > return ret; > } > > +static int find_ram_target(off_t *phys_addr, > + unsigned long pagesize) > +{ > + unsigned long long start, end; > + char line[80], *end_ptr; > + FILE *file; > + > + /* Search /proc/iomem for the first suitable "System RAM" range. */ > + file = fopen("/proc/iomem", "r"); > + if (!file) > + return -errno; > + > + while (fgets(line, sizeof(line), file)) { > + /* Ignore any child nodes. */ > + if (!isalnum(line[0])) > + continue; > + > + if (!strstr(line, "System RAM\n")) > + continue; > + > + start = strtoull(line, &end_ptr, 16); > + /* Skip over the "-" */ > + end_ptr++; > + /* Make end "exclusive". */ > + end = strtoull(end_ptr, NULL, 16) + 1; > + > + /* Actual addresses are not exported */ > + if (!start && !end) > + break; > + > + /* We need full pages. */ > + start = (start + pagesize - 1) & ~(pagesize - 1); > + end &= ~(pagesize - 1); > + > + if (start != (off_t)start) > + break; > + > + /* We need two pages. */ > + if (end > start + 2 * pagesize) { > +
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28.05.25 12:53, Ryan Roberts wrote: On 28/05/2025 11:48, David Hildenbrand wrote: On 28.05.25 12:44, David Hildenbrand wrote: On 28.05.25 12:34, Ryan Roberts wrote: Hi David, On 09/05/2025 16:30, 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. We are seeing really horrible RAS errors with this test when run on arm64 tx2 machine. Based solely on reviewing the code, I think the problem is that tx2 doesn't have anything at phys address 0, so test_read_access() is trying to put trasactions out to a bad address on the bus. tx2 /proc/iomem: $ sudo cat /proc/iomem 3000-37ff : PCI ECAM 3800-3fff : PCI ECAM 4000-5fff : PCI Bus :00 ... Whereas my x86 box has some reserved memory: $ sudo cat /proc/iomem -0fff : Reserved 1000-0003dfff : System RAM ... A quick fix would be to make this test specific to x86 (the only one I tested on). We should always have the lower two pages IIRC (BIOS stuff etc). I'm not sure how far along this patch is? I'm guessing mm-stable? Perhaps you can do the quick fix, then I'd be happy to make this more robust for arm64 later? Can you give the following a quick test on that machine? Then, I can send it as a proper patch later. From 40fea063f2fcf1474fb47cb9aebdb04fd825032b Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Wed, 28 May 2025 14:35:23 +0200 Subject: [PATCH] selftests/mm: two fixes for the pfnmap test When unregistering the signal handler, we have to pass SIG_DFL, and blindly reading from PFN 0 and PFN 1 seems to be problematic on !x86 systems. In particularly, on arm64 tx2 machines where noting resides at these physical memory locations, we can generate RAS errors. Let's fix it by scanning /proc/iomem for actual "System RAM". Reported-by: Ryan Roberts Closes: https://lore.kernel.org/all/[email protected]/T/#u Fixes: 2616b370323a ("selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem") Signed-off-by: David Hildenbrand --- tools/testing/selftests/mm/pfnmap.c | 61 +++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c index 8a9d19b6020c7..4943927a7d1ea 100644 --- a/tools/testing/selftests/mm/pfnmap.c +++ b/tools/testing/selftests/mm/pfnmap.c @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include #include @@ -43,14 +45,62 @@ static int test_read_access(char *addr, size_t size, size_t pagesize) /* Force a read that the compiler cannot optimize out. */ *((volatile char *)(addr + offs)); } - if (signal(SIGSEGV, signal_handler) == SIG_ERR) + if (signal(SIGSEGV, SIG_DFL) == SIG_ERR) return -EINVAL; return ret; } +static int find_ram_target(off_t *phys_addr, + unsigned long pagesize) +{ + unsigned long long start, end; + char line[80], *end_ptr; + FILE *file; + + /* Search /proc/iomem for the first suitable "System RAM" range. */ + file = fopen("/proc/iomem", "r"); + if (!file) + return -errno; + + while (fgets(line, sizeof(line), file)) { + /* Ignore any child nodes. */ + if (!isalnum(line[0])) + continue; + + if (!strstr(line, "System RAM\n")) + continue; + + start = strtoull(line, &end_ptr, 16); + /* Skip over the "-" */ + end_ptr++; + /* Make end "exclusive". */ + end = strtoull(end_ptr, NULL, 16) + 1; + + /* Actual addresses are not exported */ + if (!start && !end) + break; + + /* We need full pages. */ + start = (start + pagesize - 1) & ~(pagesize - 1); + end &= ~(pagesize - 1); + + if (start != (off_t)start) + break; + + /* We need two pages. */ + if (end > start + 2 * pagesize) { + fclose(file); + *phys_addr = start; + return 0; + } + } + return -ENOENT; +} + FIXTURE(pfnmap) { + off_t phys_addr; size_t pagesize; int dev_mem_fd; char *addr1; @@ -63,14 +113,17 @@ FIXTURE_SETUP(pfnmap) { self->pagesize = getpagesize(); + /* We'll require two physical pages throughout our tests ... */ + if (find_ram_target(&self->phys_addr, self->pagesize)) + SKIP(return, "Cannot find ram target in '/dev/iomem'\n"); + self->dev_
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28/05/2025 11:48, David Hildenbrand wrote: > On 28.05.25 12:44, David Hildenbrand wrote: >> On 28.05.25 12:34, Ryan Roberts wrote: >>> Hi David, >>> >>> >>> On 09/05/2025 16:30, 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. >>> >>> We are seeing really horrible RAS errors with this test when run on arm64 >>> tx2 >>> machine. Based solely on reviewing the code, I think the problem is that tx2 >>> doesn't have anything at phys address 0, so test_read_access() is trying to >>> put >>> trasactions out to a bad address on the bus. >>> >>> tx2 /proc/iomem: >>> >>> $ sudo cat /proc/iomem >>> 3000-37ff : PCI ECAM >>> 3800-3fff : PCI ECAM >>> 4000-5fff : PCI Bus :00 >>> ... >>> >>> Whereas my x86 box has some reserved memory: >>> >>> $ sudo cat /proc/iomem >>> -0fff : Reserved >>> 1000-0003dfff : System RAM >>> ... >>> >> >> A quick fix would be to make this test specific to x86 (the only one I >> tested on). We should always have the lower two pages IIRC (BIOS stuff etc). I'm not sure how far along this patch is? I'm guessing mm-stable? Perhaps you can do the quick fix, then I'd be happy to make this more robust for arm64 later? >> >>> I think perhaps the only safe way to handle this is to parse /proc/iomem >>> for a >>> region of "System RAM" that is at least 2 pages then use that for your read >>> tests. This would also solve the hypothetical issue of reading something >>> that >>> has read size effects. >> >> That sounds also plausible yes. I somehow remembered that mmap() would >> fail if "there is nothing". > > Ah, my memory comes back, we perform checks only with CONFIG_STRICT_DEVMEM. Ahh makes sense. I guess our config doesn't include this. I just checked the RAS error and it is for PA 0. So I'm confident that what I describe above is definitely what is happening.
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28.05.25 12:44, David Hildenbrand wrote: On 28.05.25 12:34, Ryan Roberts wrote: Hi David, On 09/05/2025 16:30, 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. We are seeing really horrible RAS errors with this test when run on arm64 tx2 machine. Based solely on reviewing the code, I think the problem is that tx2 doesn't have anything at phys address 0, so test_read_access() is trying to put trasactions out to a bad address on the bus. tx2 /proc/iomem: $ sudo cat /proc/iomem 3000-37ff : PCI ECAM 3800-3fff : PCI ECAM 4000-5fff : PCI Bus :00 ... Whereas my x86 box has some reserved memory: $ sudo cat /proc/iomem -0fff : Reserved 1000-0003dfff : System RAM ... A quick fix would be to make this test specific to x86 (the only one I tested on). We should always have the lower two pages IIRC (BIOS stuff etc). I think perhaps the only safe way to handle this is to parse /proc/iomem for a region of "System RAM" that is at least 2 pages then use that for your read tests. This would also solve the hypothetical issue of reading something that has read size effects. That sounds also plausible yes. I somehow remembered that mmap() would fail if "there is nothing". Ah, my memory comes back, we perform checks only with CONFIG_STRICT_DEVMEM. -- Cheers, David / dhildenb
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28.05.25 12:53, Ryan Roberts wrote: On 28/05/2025 11:48, David Hildenbrand wrote: On 28.05.25 12:44, David Hildenbrand wrote: On 28.05.25 12:34, Ryan Roberts wrote: Hi David, On 09/05/2025 16:30, 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. We are seeing really horrible RAS errors with this test when run on arm64 tx2 machine. Based solely on reviewing the code, I think the problem is that tx2 doesn't have anything at phys address 0, so test_read_access() is trying to put trasactions out to a bad address on the bus. tx2 /proc/iomem: $ sudo cat /proc/iomem 3000-37ff : PCI ECAM 3800-3fff : PCI ECAM 4000-5fff : PCI Bus :00 ... Whereas my x86 box has some reserved memory: $ sudo cat /proc/iomem -0fff : Reserved 1000-0003dfff : System RAM ... A quick fix would be to make this test specific to x86 (the only one I tested on). We should always have the lower two pages IIRC (BIOS stuff etc). I'm not sure how far along this patch is? I'm guessing mm-stable? Perhaps you can do the quick fix, then I'd be happy to make this more robust for arm64 later? Already hacking on the parsing :) -- Cheers, David / dhildenb
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 28.05.25 12:34, Ryan Roberts wrote: Hi David, On 09/05/2025 16:30, 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. We are seeing really horrible RAS errors with this test when run on arm64 tx2 machine. Based solely on reviewing the code, I think the problem is that tx2 doesn't have anything at phys address 0, so test_read_access() is trying to put trasactions out to a bad address on the bus. tx2 /proc/iomem: $ sudo cat /proc/iomem 3000-37ff : PCI ECAM 3800-3fff : PCI ECAM 4000-5fff : PCI Bus :00 ... Whereas my x86 box has some reserved memory: $ sudo cat /proc/iomem -0fff : Reserved 1000-0003dfff : System RAM ... A quick fix would be to make this test specific to x86 (the only one I tested on). We should always have the lower two pages IIRC (BIOS stuff etc). I think perhaps the only safe way to handle this is to parse /proc/iomem for a region of "System RAM" that is at least 2 pages then use that for your read tests. This would also solve the hypothetical issue of reading something that has read size effects. That sounds also plausible yes. I somehow remembered that mmap() would fail if "there is nothing". I also spotted a few nits while reading the code... On current x86-64 with PAT inside a VM, all tests pass: TAP version 13 1..6 # Starting 6 tests from 1 test cases. # RUN pfnmap.madvise_disallowed ... #OK pfnmap.madvise_disallowed ok 1 pfnmap.madvise_disallowed # RUN pfnmap.munmap_split ... #OK pfnmap.munmap_split ok 2 pfnmap.munmap_split # RUN pfnmap.mremap_fixed ... #OK pfnmap.mremap_fixed ok 3 pfnmap.mremap_fixed # RUN pfnmap.mremap_shrink ... #OK pfnmap.mremap_shrink ok 4 pfnmap.mremap_shrink # RUN pfnmap.mremap_expand ... #OK pfnmap.mremap_expand ok 5 pfnmap.mremap_expand # RUN pfnmap.fork ... #OK pfnmap.fork ok 6 pfnmap.fork # PASSED: 6 / 6 tests passed. # Totals: pass:6 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 0x-0x0fff] >> 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 Cc: Shuah Khan Cc: Lorenzo Stoakes Cc: Ingo Molnar Cc: Peter Xu Cc: Dev Jain Signed-off-by: David Hildenbrand --- Hopefully I didn't miss any review feedback. v1 -> v2: * Rewrite using kselftest_harness, which simplifies a lot of things * Add to .gitignore and run_vmtests.sh * Register signal handler on demand * Use volatile trick to force a read (not factoring out FORCE_READ just yet) * Drop mprotect() test case * Add some more comments why we test certain things * Use NULL for mmap() first parameter instead of 0 * Smaller fixes --- tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 1 + tools/testing/selftests/mm/pfnmap.c | 196 ++ tools/testing/selftests/mm/run_vmtests.sh | 4 + 4 files changed, 202 insertions(+) create mode 100644 tools/testing/selftests/mm/pfnmap.c diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 91db34941a143..824266982aa36 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -20,6 +20,7 @@ mremap_test on-fault-limit transhuge-stress pagemap_ioctl +pfnmap *.tmp* protection_keys protection_keys_32 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 0..8a9d19b6020c7 --- /dev/null +++ b/tools/testing/selftests/mm/pfnmap.c @@ -0,0 +1,196 @@ +// 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 + */ +#defi
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
Hi David, On 09/05/2025 16:30, 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. We are seeing really horrible RAS errors with this test when run on arm64 tx2 machine. Based solely on reviewing the code, I think the problem is that tx2 doesn't have anything at phys address 0, so test_read_access() is trying to put trasactions out to a bad address on the bus. tx2 /proc/iomem: $ sudo cat /proc/iomem 3000-37ff : PCI ECAM 3800-3fff : PCI ECAM 4000-5fff : PCI Bus :00 ... Whereas my x86 box has some reserved memory: $ sudo cat /proc/iomem -0fff : Reserved 1000-0003dfff : System RAM ... I think perhaps the only safe way to handle this is to parse /proc/iomem for a region of "System RAM" that is at least 2 pages then use that for your read tests. This would also solve the hypothetical issue of reading something that has read size effects. I also spotted a few nits while reading the code... > > On current x86-64 with PAT inside a VM, all tests pass: > > TAP version 13 > 1..6 > # Starting 6 tests from 1 test cases. > # RUN pfnmap.madvise_disallowed ... > #OK pfnmap.madvise_disallowed > ok 1 pfnmap.madvise_disallowed > # RUN pfnmap.munmap_split ... > #OK pfnmap.munmap_split > ok 2 pfnmap.munmap_split > # RUN pfnmap.mremap_fixed ... > #OK pfnmap.mremap_fixed > ok 3 pfnmap.mremap_fixed > # RUN pfnmap.mremap_shrink ... > #OK pfnmap.mremap_shrink > ok 4 pfnmap.mremap_shrink > # RUN pfnmap.mremap_expand ... > #OK pfnmap.mremap_expand > ok 5 pfnmap.mremap_expand > # RUN pfnmap.fork ... > #OK pfnmap.fork > ok 6 pfnmap.fork > # PASSED: 6 / 6 tests passed. > # Totals: pass:6 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 > 0x-0x0fff] > > 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 > Cc: Shuah Khan > Cc: Lorenzo Stoakes > Cc: Ingo Molnar > Cc: Peter Xu > Cc: Dev Jain > Signed-off-by: David Hildenbrand > --- > > Hopefully I didn't miss any review feedback. > > v1 -> v2: > * Rewrite using kselftest_harness, which simplifies a lot of things > * Add to .gitignore and run_vmtests.sh > * Register signal handler on demand > * Use volatile trick to force a read (not factoring out FORCE_READ just yet) > * Drop mprotect() test case > * Add some more comments why we test certain things > * Use NULL for mmap() first parameter instead of 0 > * Smaller fixes > > --- > tools/testing/selftests/mm/.gitignore | 1 + > tools/testing/selftests/mm/Makefile | 1 + > tools/testing/selftests/mm/pfnmap.c | 196 ++ > tools/testing/selftests/mm/run_vmtests.sh | 4 + > 4 files changed, 202 insertions(+) > create mode 100644 tools/testing/selftests/mm/pfnmap.c > > diff --git a/tools/testing/selftests/mm/.gitignore > b/tools/testing/selftests/mm/.gitignore > index 91db34941a143..824266982aa36 100644 > --- a/tools/testing/selftests/mm/.gitignore > +++ b/tools/testing/selftests/mm/.gitignore > @@ -20,6 +20,7 @@ mremap_test > on-fault-limit > transhuge-stress > pagemap_ioctl > +pfnmap > *.tmp* > protection_keys > protection_keys_32 > 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 0..8a9d19b6020c7 > --- /dev/null > +++ b/tools/testing/selftests/mm/pfnmap.c > @@ -0,0 +1,196 @@ > +// 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 > + */ > +#define _GNU_SOURCE > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On Mon, May 12, 2025 at 10:18:05AM +0200, David Hildenbrand wrote: > On 09.05.25 17:55, Lorenzo Stoakes wrote: > > On Fri, May 09, 2025 at 05:30:32PM +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. > > > > > > 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..6 > > > # Starting 6 tests from 1 test cases. > > > # RUN pfnmap.madvise_disallowed ... > > > #OK pfnmap.madvise_disallowed > > > ok 1 pfnmap.madvise_disallowed > > > # RUN pfnmap.munmap_split ... > > > #OK pfnmap.munmap_split > > > ok 2 pfnmap.munmap_split > > > # RUN pfnmap.mremap_fixed ... > > > #OK pfnmap.mremap_fixed > > > ok 3 pfnmap.mremap_fixed > > > # RUN pfnmap.mremap_shrink ... > > > #OK pfnmap.mremap_shrink > > > ok 4 pfnmap.mremap_shrink > > > # RUN pfnmap.mremap_expand ... > > > #OK pfnmap.mremap_expand > > > ok 5 pfnmap.mremap_expand > > > # RUN pfnmap.fork ... > > > #OK pfnmap.fork > > > ok 6 pfnmap.fork > > > # PASSED: 6 / 6 tests passed. > > > # Totals: pass:6 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 > > > 0x-0x0fff] > > > > > > 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 > > > Cc: Shuah Khan > > > Cc: Lorenzo Stoakes > > > Cc: Ingo Molnar > > > Cc: Peter Xu > > > Cc: Dev Jain > > > Signed-off-by: David Hildenbrand > > > > Nice, big improvement! > > > > Reviewed-by: Lorenzo Stoakes > > Thanks! It was worth spending the time on using the harness. > > The FIXTURE_TEARDOWN() stuff is really confusing. It's not actually required > to teardown most stuff (unless you create files in setup etc), because all > tests are executed in a fork'ed child, where fd's, mappings, ... will go > away immediately afterwards during the exit(). Yeah, it's maybe not always necessary, but stil handy, and at least allows for strict cleanup/separation between tests. And having things structured this way makes life much easier in many other regards, as you have one place for it, you don't have to manually fiddle with test counts etc. etc. Overall I think it's a big win :) > > I still implemented FIXTURE_TEARDOWN (like everybody else), because maybe > the manual teardown can find other issues not triggered during exit(). Ack! > > -- > Cheers, > > David / dhildenb > Cheers, Lorenzo
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On 09.05.25 17:55, Lorenzo Stoakes wrote: On Fri, May 09, 2025 at 05:30:32PM +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. 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..6 # Starting 6 tests from 1 test cases. # RUN pfnmap.madvise_disallowed ... #OK pfnmap.madvise_disallowed ok 1 pfnmap.madvise_disallowed # RUN pfnmap.munmap_split ... #OK pfnmap.munmap_split ok 2 pfnmap.munmap_split # RUN pfnmap.mremap_fixed ... #OK pfnmap.mremap_fixed ok 3 pfnmap.mremap_fixed # RUN pfnmap.mremap_shrink ... #OK pfnmap.mremap_shrink ok 4 pfnmap.mremap_shrink # RUN pfnmap.mremap_expand ... #OK pfnmap.mremap_expand ok 5 pfnmap.mremap_expand # RUN pfnmap.fork ... #OK pfnmap.fork ok 6 pfnmap.fork # PASSED: 6 / 6 tests passed. # Totals: pass:6 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 0x-0x0fff] 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 Cc: Shuah Khan Cc: Lorenzo Stoakes Cc: Ingo Molnar Cc: Peter Xu Cc: Dev Jain Signed-off-by: David Hildenbrand Nice, big improvement! Reviewed-by: Lorenzo Stoakes Thanks! It was worth spending the time on using the harness. The FIXTURE_TEARDOWN() stuff is really confusing. It's not actually required to teardown most stuff (unless you create files in setup etc), because all tests are executed in a fork'ed child, where fd's, mappings, ... will go away immediately afterwards during the exit(). I still implemented FIXTURE_TEARDOWN (like everybody else), because maybe the manual teardown can find other issues not triggered during exit(). -- Cheers, David / dhildenb
Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem
On Fri, May 09, 2025 at 05:30:32PM +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.
>
> 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..6
> # Starting 6 tests from 1 test cases.
> # RUN pfnmap.madvise_disallowed ...
> #OK pfnmap.madvise_disallowed
> ok 1 pfnmap.madvise_disallowed
> # RUN pfnmap.munmap_split ...
> #OK pfnmap.munmap_split
> ok 2 pfnmap.munmap_split
> # RUN pfnmap.mremap_fixed ...
> #OK pfnmap.mremap_fixed
> ok 3 pfnmap.mremap_fixed
> # RUN pfnmap.mremap_shrink ...
> #OK pfnmap.mremap_shrink
> ok 4 pfnmap.mremap_shrink
> # RUN pfnmap.mremap_expand ...
> #OK pfnmap.mremap_expand
> ok 5 pfnmap.mremap_expand
> # RUN pfnmap.fork ...
> #OK pfnmap.fork
> ok 6 pfnmap.fork
> # PASSED: 6 / 6 tests passed.
> # Totals: pass:6 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
> 0x-0x0fff]
>
> 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
> Cc: Shuah Khan
> Cc: Lorenzo Stoakes
> Cc: Ingo Molnar
> Cc: Peter Xu
> Cc: Dev Jain
> Signed-off-by: David Hildenbrand
Nice, big improvement!
Reviewed-by: Lorenzo Stoakes
> ---
>
> Hopefully I didn't miss any review feedback.
All good afaict! :) the only thing would be to make the siglongjmp() conditional
but it's not a big deal.
>
> v1 -> v2:
> * Rewrite using kselftest_harness, which simplifies a lot of things
> * Add to .gitignore and run_vmtests.sh
> * Register signal handler on demand
> * Use volatile trick to force a read (not factoring out FORCE_READ just yet)
> * Drop mprotect() test case
> * Add some more comments why we test certain things
> * Use NULL for mmap() first parameter instead of 0
> * Smaller fixes
>
> ---
> tools/testing/selftests/mm/.gitignore | 1 +
> tools/testing/selftests/mm/Makefile | 1 +
> tools/testing/selftests/mm/pfnmap.c | 196 ++
> tools/testing/selftests/mm/run_vmtests.sh | 4 +
> 4 files changed, 202 insertions(+)
> create mode 100644 tools/testing/selftests/mm/pfnmap.c
>
> diff --git a/tools/testing/selftests/mm/.gitignore
> b/tools/testing/selftests/mm/.gitignore
> index 91db34941a143..824266982aa36 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -20,6 +20,7 @@ mremap_test
> on-fault-limit
> transhuge-stress
> pagemap_ioctl
> +pfnmap
> *.tmp*
> protection_keys
> protection_keys_32
> 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 0..8a9d19b6020c7
> --- /dev/null
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -0,0 +1,196 @@
> +// 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
> + */
> +#define _GNU_SOURCE
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "../kselftest_harness.h"
> +#include "vm_util.h"
> +
> +static sigjmp_buf sigjmp_buf_env;
> +
> +static void signal_handler(int sig)
> +{
> + siglongjmp(sigjmp_buf_env, -EFAULT);
> +}
> +
> +static int test_read_access(char *addr, size_t size, size_t pagesize)
> +{
> + size_t offs;
> + int ret;
> +
> + if (signal(SIGSEGV, signal_handler) == SIG_ERR)
> + return -EINVAL;
> +
> + ret = sigsetjmp(sigjmp_buf_env, 1);
> + if (!ret) {
> + for (offs = 0; offs < size; offs += pagesize)
> + /* Force a read that the compiler cannot optimize out.
> */
> + *((volatile char *)(addr + of
