pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
meaning once for every test, and skips the test if any check fails.
The target file is the same for every test so this is a little
overkill. More importantly, this approach means that the whole suite
will report PASS even if all the tests are skipped because kernel
configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
being mapped, for instance.
Let's ensure that KSFT_SKIP is returned as exit code if any check
fails by performing the checks in pfnmap_init(), run once. That
function also takes care of finding the offset of the pages to be
mapped and saves it in a global. The file is still mapped/unmapped
for every test, as some of them modify the mapping.
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
1 file changed, 55 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
index 35b0e3ed54cd..e41d5464130b 100644
--- a/tools/testing/selftests/mm/pfnmap.c
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -25,8 +25,11 @@
#include "kselftest_harness.h"
#include "vm_util.h"
+#define DEV_MEM_NPAGES 2
+
static sigjmp_buf sigjmp_buf_env;
static char *file = "/dev/mem";
+static off_t file_offset;
static void signal_handler(int sig)
{
@@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
break;
/* We need two pages. */
- if (end > start + 2 * pagesize) {
+ if (end > start + DEV_MEM_NPAGES * pagesize) {
fclose(file);
*offset = start;
return 0;
@@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
return -ENOENT;
}
+static void pfnmap_init(void)
+{
+ size_t pagesize = getpagesize();
+ size_t size = DEV_MEM_NPAGES * pagesize;
+ int fd;
+ void *addr;
+
+ if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
+ int err = find_ram_target(&file_offset, pagesize);
+
+ if (err)
+ ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
+ strerror(-err));
+ } else {
+ file_offset = 0;
+ }
+
+ /*
+ * Make sure we can open and map the file, and perform some basic
+ * checks; skip the whole suite if anything goes wrong.
+ * A fresh mapping is then created for every test case by
+ * FIXTURE_SETUP(pfnmap).
+ */
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
+
+ addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
+ if (addr == MAP_FAILED)
+ ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
+
+ if (!check_vmflag_pfnmap(addr))
+ ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
+
+ if (test_read_access(addr, size))
+ ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
+
+ munmap(addr, size);
+ close(fd);
+}
+
FIXTURE(pfnmap)
{
- off_t offset;
size_t pagesize;
int dev_mem_fd;
char *addr1;
@@ -112,31 +155,13 @@ FIXTURE_SETUP(pfnmap)
{
self->pagesize = getpagesize();
- if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
- /* We'll require two physical pages throughout our tests ... */
- if (find_ram_target(&self->offset, self->pagesize))
- SKIP(return,
- "Cannot find ram target in '/proc/iomem'\n");
- } else {
- self->offset = 0;
- }
-
self->dev_mem_fd = open(file, O_RDONLY);
- if (self->dev_mem_fd < 0)
- SKIP(return, "Cannot open '%s'\n", file);
+ ASSERT_GE(self->dev_mem_fd, 0);
- self->size1 = self->pagesize * 2;
+ self->size1 = DEV_MEM_NPAGES * self->pagesize;
self->addr1 = mmap(NULL, self->size1, PROT_READ, MAP_SHARED,
- self->dev_mem_fd, self->offset);
- if (self->addr1 == MAP_FAILED)
- SKIP(return, "Cannot mmap '%s'\n", file);
-
- if (!check_vmflag_pfnmap(self->addr1))
- SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
-
- /* ... and want to be able to read from them. */
- if (test_read_access(self->addr1, self->size1))
- SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
+ self->dev_mem_fd, file_offset);
+ ASSERT_NE(self->addr1, MAP_FAILED);
self->size2 = 0;
self->addr2 = MAP_FAILED;
@@ -189,7 +214,7 @@ TEST_F(pfnmap, munmap_split)
*/
self->size2 = self->pagesize;
self->addr2 = mmap(NULL, self->pagesize, PROT_READ, MAP_SHARED,
- self->dev_mem_fd, self->offset);
+ self->dev_mem_fd, file_offset);
ASSERT_NE(self->addr2, MAP_FAILED);
}
@@ -258,8 +283,12 @@ int main(int argc, char **argv)
if (strcmp(argv[i], "--") == 0) {
if (i + 1 < argc && strlen(argv[i + 1]) > 0)
file = argv[i + 1];
- return test_harness_run(i, argv);
+ argc = i;
+ break;
}
}
+
+ pfnmap_init();
+
return test_harness_run(argc, argv);
}
--
2.51.2On 1/7/26 17:48, Kevin Brodsky wrote:
> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
> meaning once for every test, and skips the test if any check fails.
>
> The target file is the same for every test so this is a little
> overkill. More importantly, this approach means that the whole suite
> will report PASS even if all the tests are skipped because kernel
> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
> being mapped, for instance.
>
> Let's ensure that KSFT_SKIP is returned as exit code if any check
> fails by performing the checks in pfnmap_init(), run once. That
> function also takes care of finding the offset of the pages to be
> mapped and saves it in a global. The file is still mapped/unmapped
> for every test, as some of them modify the mapping.
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
> tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
> 1 file changed, 55 insertions(+), 26 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index 35b0e3ed54cd..e41d5464130b 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -25,8 +25,11 @@
> #include "kselftest_harness.h"
> #include "vm_util.h"
>
> +#define DEV_MEM_NPAGES 2
> +
> static sigjmp_buf sigjmp_buf_env;
> static char *file = "/dev/mem";
> +static off_t file_offset;
>
> static void signal_handler(int sig)
> {
> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
> break;
>
> /* We need two pages. */
> - if (end > start + 2 * pagesize) {
> + if (end > start + DEV_MEM_NPAGES * pagesize) {
> fclose(file);
> *offset = start;
> return 0;
> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
> return -ENOENT;
> }
>
> +static void pfnmap_init(void)
> +{
> + size_t pagesize = getpagesize();
> + size_t size = DEV_MEM_NPAGES * pagesize;
> + int fd;
> + void *addr;
> +
> + if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> + int err = find_ram_target(&file_offset, pagesize);
> +
> + if (err)
> + ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
> + strerror(-err));
> + } else {
> + file_offset = 0;
> + }
> +
> + /*
> + * Make sure we can open and map the file, and perform some basic
> + * checks; skip the whole suite if anything goes wrong.
> + * A fresh mapping is then created for every test case by
> + * FIXTURE_SETUP(pfnmap).
> + */
> + fd = open(file, O_RDONLY);
> + if (fd < 0)
> + ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
> +
> + addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
> + if (addr == MAP_FAILED)
> + ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
> +
> + if (!check_vmflag_pfnmap(addr))
> + ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
> +
> + if (test_read_access(addr, size))
> + ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
> +
> + munmap(addr, size);
Why not keep the fd open then and supply that to all tests without the
need for them to open/close?
Then, also the file cannot change etc.
--
Cheers
David
On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, Kevin Brodsky wrote:
>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>> meaning once for every test, and skips the test if any check fails.
>>
>> The target file is the same for every test so this is a little
>> overkill. More importantly, this approach means that the whole suite
>> will report PASS even if all the tests are skipped because kernel
>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>> being mapped, for instance.
>>
>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>> fails by performing the checks in pfnmap_init(), run once. That
>> function also takes care of finding the offset of the pages to be
>> mapped and saves it in a global. The file is still mapped/unmapped
>> for every test, as some of them modify the mapping.
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>> tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>> 1 file changed, 55 insertions(+), 26 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>> pfnmap.c
>> index 35b0e3ed54cd..e41d5464130b 100644
>> --- a/tools/testing/selftests/mm/pfnmap.c
>> +++ b/tools/testing/selftests/mm/pfnmap.c
>> @@ -25,8 +25,11 @@
>> #include "kselftest_harness.h"
>> #include "vm_util.h"
>> +#define DEV_MEM_NPAGES 2
>> +
>> static sigjmp_buf sigjmp_buf_env;
>> static char *file = "/dev/mem";
>> +static off_t file_offset;
>> static void signal_handler(int sig)
>> {
>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>> break;
>> /* We need two pages. */
>> - if (end > start + 2 * pagesize) {
>> + if (end > start + DEV_MEM_NPAGES * pagesize) {
>> fclose(file);
>> *offset = start;
>> return 0;
>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>> return -ENOENT;
>> }
>> +static void pfnmap_init(void)
>> +{
>> + size_t pagesize = getpagesize();
>> + size_t size = DEV_MEM_NPAGES * pagesize;
>> + int fd;
>> + void *addr;
>> +
>> + if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>> + int err = find_ram_target(&file_offset, pagesize);
>> +
>> + if (err)
>> + ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>> + strerror(-err));
>> + } else {
>> + file_offset = 0;
>> + }
>> +
>> + /*
>> + * Make sure we can open and map the file, and perform some basic
>> + * checks; skip the whole suite if anything goes wrong.
>> + * A fresh mapping is then created for every test case by
>> + * FIXTURE_SETUP(pfnmap).
>> + */
>> + fd = open(file, O_RDONLY);
>> + if (fd < 0)
>> + ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>> +
>> + addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>> + if (addr == MAP_FAILED)
>> + ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>> +
>> + if (!check_vmflag_pfnmap(addr))
>> + ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>> +
>> + if (test_read_access(addr, size))
>> + ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>> +
>> + munmap(addr, size);
>
> Why not keep the fd open then and supply that to all tests without the need for
> them to open/close?
>
> Then, also the file cannot change etc.
I had a private conversation with Kevin about this before he posted; my very
minor, theorectical concern about that was that it's possible to pass in a
custom file to be pfnmapped and I wondered if such a file could map a device
region that has read side effects? In that case I think you'd want to open it
fresh for each test to ensure consistent starting state?
But if you think that concern is unfounded, certainly just opening it once and
reusing will simplify.
Thanks,
Ryan
On 1/19/26 15:26, Ryan Roberts wrote:
> On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
>> On 1/7/26 17:48, Kevin Brodsky wrote:
>>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>>> meaning once for every test, and skips the test if any check fails.
>>>
>>> The target file is the same for every test so this is a little
>>> overkill. More importantly, this approach means that the whole suite
>>> will report PASS even if all the tests are skipped because kernel
>>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>>> being mapped, for instance.
>>>
>>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>>> fails by performing the checks in pfnmap_init(), run once. That
>>> function also takes care of finding the offset of the pages to be
>>> mapped and saves it in a global. The file is still mapped/unmapped
>>> for every test, as some of them modify the mapping.
>>>
>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>> ---
>>> tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>>> 1 file changed, 55 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>>> pfnmap.c
>>> index 35b0e3ed54cd..e41d5464130b 100644
>>> --- a/tools/testing/selftests/mm/pfnmap.c
>>> +++ b/tools/testing/selftests/mm/pfnmap.c
>>> @@ -25,8 +25,11 @@
>>> #include "kselftest_harness.h"
>>> #include "vm_util.h"
>>> +#define DEV_MEM_NPAGES 2
>>> +
>>> static sigjmp_buf sigjmp_buf_env;
>>> static char *file = "/dev/mem";
>>> +static off_t file_offset;
>>> static void signal_handler(int sig)
>>> {
>>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>>> break;
>>> /* We need two pages. */
>>> - if (end > start + 2 * pagesize) {
>>> + if (end > start + DEV_MEM_NPAGES * pagesize) {
>>> fclose(file);
>>> *offset = start;
>>> return 0;
>>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>>> return -ENOENT;
>>> }
>>> +static void pfnmap_init(void)
>>> +{
>>> + size_t pagesize = getpagesize();
>>> + size_t size = DEV_MEM_NPAGES * pagesize;
>>> + int fd;
>>> + void *addr;
>>> +
>>> + if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>>> + int err = find_ram_target(&file_offset, pagesize);
>>> +
>>> + if (err)
>>> + ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>>> + strerror(-err));
>>> + } else {
>>> + file_offset = 0;
>>> + }
>>> +
>>> + /*
>>> + * Make sure we can open and map the file, and perform some basic
>>> + * checks; skip the whole suite if anything goes wrong.
>>> + * A fresh mapping is then created for every test case by
>>> + * FIXTURE_SETUP(pfnmap).
>>> + */
>>> + fd = open(file, O_RDONLY);
>>> + if (fd < 0)
>>> + ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>>> +
>>> + addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>>> + if (addr == MAP_FAILED)
>>> + ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>>> +
>>> + if (!check_vmflag_pfnmap(addr))
>>> + ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>>> +
>>> + if (test_read_access(addr, size))
>>> + ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>>> +
>>> + munmap(addr, size);
>>
>> Why not keep the fd open then and supply that to all tests without the need for
>> them to open/close?
>>
>> Then, also the file cannot change etc.
>
> I had a private conversation with Kevin about this before he posted; my very
> minor, theorectical concern about that was that it's possible to pass in a
> custom file to be pfnmapped and I wondered if such a file could map a device
> region that has read side effects? In that case I think you'd want to open it
> fresh for each test to ensure consistent starting state?
Are we aware of devices where we would actually require a new open, and
not just a new mmap()?
The reason we added support for other files was "other pfnmap'ed memory
like NVIDIA's EGM". I'd assume that people rather should not pass in
something that has any side-effects.
>
> But if you think that concern is unfounded, certainly just opening it once and
> reusing will simplify.
I would just keep it simple here, yes. If this ever becomes a real
problem, my intuition would tell me that probably the caller is doing
something unsupported that we just cannot easily identify+reject.
--
Cheers
David
On 19/01/2026 14:32, David Hildenbrand (Red Hat) wrote:
> On 1/19/26 15:26, Ryan Roberts wrote:
>> On 19/01/2026 11:16, David Hildenbrand (Red Hat) wrote:
>>> On 1/7/26 17:48, Kevin Brodsky wrote:
>>>> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
>>>> meaning once for every test, and skips the test if any check fails.
>>>>
>>>> The target file is the same for every test so this is a little
>>>> overkill. More importantly, this approach means that the whole suite
>>>> will report PASS even if all the tests are skipped because kernel
>>>> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
>>>> being mapped, for instance.
>>>>
>>>> Let's ensure that KSFT_SKIP is returned as exit code if any check
>>>> fails by performing the checks in pfnmap_init(), run once. That
>>>> function also takes care of finding the offset of the pages to be
>>>> mapped and saves it in a global. The file is still mapped/unmapped
>>>> for every test, as some of them modify the mapping.
>>>>
>>>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>>>> ---
>>>> tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
>>>> 1 file changed, 55 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/
>>>> pfnmap.c
>>>> index 35b0e3ed54cd..e41d5464130b 100644
>>>> --- a/tools/testing/selftests/mm/pfnmap.c
>>>> +++ b/tools/testing/selftests/mm/pfnmap.c
>>>> @@ -25,8 +25,11 @@
>>>> #include "kselftest_harness.h"
>>>> #include "vm_util.h"
>>>> +#define DEV_MEM_NPAGES 2
>>>> +
>>>> static sigjmp_buf sigjmp_buf_env;
>>>> static char *file = "/dev/mem";
>>>> +static off_t file_offset;
>>>> static void signal_handler(int sig)
>>>> {
>>>> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
>>>> break;
>>>> /* We need two pages. */
>>>> - if (end > start + 2 * pagesize) {
>>>> + if (end > start + DEV_MEM_NPAGES * pagesize) {
>>>> fclose(file);
>>>> *offset = start;
>>>> return 0;
>>>> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
>>>> return -ENOENT;
>>>> }
>>>> +static void pfnmap_init(void)
>>>> +{
>>>> + size_t pagesize = getpagesize();
>>>> + size_t size = DEV_MEM_NPAGES * pagesize;
>>>> + int fd;
>>>> + void *addr;
>>>> +
>>>> + if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
>>>> + int err = find_ram_target(&file_offset, pagesize);
>>>> +
>>>> + if (err)
>>>> + ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
>>>> + strerror(-err));
>>>> + } else {
>>>> + file_offset = 0;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Make sure we can open and map the file, and perform some basic
>>>> + * checks; skip the whole suite if anything goes wrong.
>>>> + * A fresh mapping is then created for every test case by
>>>> + * FIXTURE_SETUP(pfnmap).
>>>> + */
>>>> + fd = open(file, O_RDONLY);
>>>> + if (fd < 0)
>>>> + ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
>>>> +
>>>> + addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
>>>> + if (addr == MAP_FAILED)
>>>> + ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
>>>> +
>>>> + if (!check_vmflag_pfnmap(addr))
>>>> + ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
>>>> +
>>>> + if (test_read_access(addr, size))
>>>> + ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
>>>> +
>>>> + munmap(addr, size);
>>>
>>> Why not keep the fd open then and supply that to all tests without the need for
>>> them to open/close?
>>>
>>> Then, also the file cannot change etc.
>>
>> I had a private conversation with Kevin about this before he posted; my very
>> minor, theorectical concern about that was that it's possible to pass in a
>> custom file to be pfnmapped and I wondered if such a file could map a device
>> region that has read side effects? In that case I think you'd want to open it
>> fresh for each test to ensure consistent starting state?
>
> Are we aware of devices where we would actually require a new open, and not just
> a new mmap()?
Nope; as I said all hypothetical. I was just being cautious.
>
> The reason we added support for other files was "other pfnmap'ed memory like
> NVIDIA's EGM". I'd assume that people rather should not pass in something that
> has any side-effects.
>
>>
>> But if you think that concern is unfounded, certainly just opening it once and
>> reusing will simplify.
>
> I would just keep it simple here, yes. If this ever becomes a real problem, my
> intuition would tell me that probably the caller is doing something unsupported
> that we just cannot easily identify+reject.
Yeah fair enough.
Thanks,
Ryan
On 20/01/2026 17:27, Ryan Roberts wrote: >>> But if you think that concern is unfounded, certainly just opening it once and >>> reusing will simplify. >> I would just keep it simple here, yes. If this ever becomes a real problem, my >> intuition would tell me that probably the caller is doing something unsupported >> that we just cannot easily identify+reject. > Yeah fair enough. Cool, will make the fd a global as well then. - Kevin
On 07/01/2026 16:48, Kevin Brodsky wrote:
> pfnmap currently checks the target file in FIXTURE_SETUP(pfnmap),
> meaning once for every test, and skips the test if any check fails.
>
> The target file is the same for every test so this is a little
> overkill. More importantly, this approach means that the whole suite
> will report PASS even if all the tests are skipped because kernel
> configuration (e.g. CONFIG_STRICT_DEVMEM=y) prevented /dev/mem from
> being mapped, for instance.
>
> Let's ensure that KSFT_SKIP is returned as exit code if any check
> fails by performing the checks in pfnmap_init(), run once. That
> function also takes care of finding the offset of the pages to be
> mapped and saves it in a global. The file is still mapped/unmapped
> for every test, as some of them modify the mapping.
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
> tools/testing/selftests/mm/pfnmap.c | 81 ++++++++++++++++++++---------
> 1 file changed, 55 insertions(+), 26 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index 35b0e3ed54cd..e41d5464130b 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -25,8 +25,11 @@
> #include "kselftest_harness.h"
> #include "vm_util.h"
>
> +#define DEV_MEM_NPAGES 2
> +
> static sigjmp_buf sigjmp_buf_env;
> static char *file = "/dev/mem";
> +static off_t file_offset;
>
> static void signal_handler(int sig)
> {
> @@ -88,7 +91,7 @@ static int find_ram_target(off_t *offset,
> break;
>
> /* We need two pages. */
> - if (end > start + 2 * pagesize) {
> + if (end > start + DEV_MEM_NPAGES * pagesize) {
> fclose(file);
> *offset = start;
> return 0;
> @@ -97,9 +100,49 @@ static int find_ram_target(off_t *offset,
> return -ENOENT;
> }
>
> +static void pfnmap_init(void)
> +{
> + size_t pagesize = getpagesize();
> + size_t size = DEV_MEM_NPAGES * pagesize;
> + int fd;
> + void *addr;
> +
> + if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> + int err = find_ram_target(&file_offset, pagesize);
> +
> + if (err)
> + ksft_exit_skip("Cannot find ram target in '/proc/iomem': %s\n",
> + strerror(-err));
> + } else {
> + file_offset = 0;
> + }
> +
> + /*
> + * Make sure we can open and map the file, and perform some basic
> + * checks; skip the whole suite if anything goes wrong.
> + * A fresh mapping is then created for every test case by
> + * FIXTURE_SETUP(pfnmap).
> + */
> + fd = open(file, O_RDONLY);
> + if (fd < 0)
> + ksft_exit_skip("Cannot open '%s': %s\n", file, strerror(errno));
> +
> + addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, file_offset);
> + if (addr == MAP_FAILED)
> + ksft_exit_skip("Cannot mmap '%s': %s\n", file, strerror(errno));
> +
> + if (!check_vmflag_pfnmap(addr))
> + ksft_exit_skip("Invalid file: '%s'. Not pfnmap'ed\n", file);
> +
> + if (test_read_access(addr, size))
> + ksft_exit_skip("Cannot read-access mmap'ed '%s'\n", file);
> +
> + munmap(addr, size);
> + close(fd);
> +}
> +
> FIXTURE(pfnmap)
> {
> - off_t offset;
> size_t pagesize;
> int dev_mem_fd;
> char *addr1;
> @@ -112,31 +155,13 @@ FIXTURE_SETUP(pfnmap)
> {
> self->pagesize = getpagesize();
>
> - if (strncmp(file, "/dev/mem", strlen("/dev/mem")) == 0) {
> - /* We'll require two physical pages throughout our tests ... */
> - if (find_ram_target(&self->offset, self->pagesize))
> - SKIP(return,
> - "Cannot find ram target in '/proc/iomem'\n");
> - } else {
> - self->offset = 0;
> - }
> -
> self->dev_mem_fd = open(file, O_RDONLY);
> - if (self->dev_mem_fd < 0)
> - SKIP(return, "Cannot open '%s'\n", file);
> + ASSERT_GE(self->dev_mem_fd, 0);
>
> - self->size1 = self->pagesize * 2;
> + self->size1 = DEV_MEM_NPAGES * self->pagesize;
> self->addr1 = mmap(NULL, self->size1, PROT_READ, MAP_SHARED,
> - self->dev_mem_fd, self->offset);
> - if (self->addr1 == MAP_FAILED)
> - SKIP(return, "Cannot mmap '%s'\n", file);
> -
> - if (!check_vmflag_pfnmap(self->addr1))
> - SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
I wonder if we still want this check per-fd, but upgraded to a fail?
Either way:
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> -
> - /* ... and want to be able to read from them. */
> - if (test_read_access(self->addr1, self->size1))
> - SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
> + self->dev_mem_fd, file_offset);
> + ASSERT_NE(self->addr1, MAP_FAILED);
>
> self->size2 = 0;
> self->addr2 = MAP_FAILED;
> @@ -189,7 +214,7 @@ TEST_F(pfnmap, munmap_split)
> */
> self->size2 = self->pagesize;
> self->addr2 = mmap(NULL, self->pagesize, PROT_READ, MAP_SHARED,
> - self->dev_mem_fd, self->offset);
> + self->dev_mem_fd, file_offset);
> ASSERT_NE(self->addr2, MAP_FAILED);
> }
>
> @@ -258,8 +283,12 @@ int main(int argc, char **argv)
> if (strcmp(argv[i], "--") == 0) {
> if (i + 1 < argc && strlen(argv[i + 1]) > 0)
> file = argv[i + 1];
> - return test_harness_run(i, argv);
> + argc = i;
> + break;
> }
> }
> +
> + pfnmap_init();
> +
> return test_harness_run(argc, argv);
> }
On 12/01/2026 10:34, Ryan Roberts wrote: >> - if (!check_vmflag_pfnmap(self->addr1)) >> - SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file); > I wonder if we still want this check per-fd, but upgraded to a fail? It seems very unlikely to fail though, I can only think of the underlying file having changed in between but I really wouldn't expect that for such special files. I was trying not to duplicate the checks too much. - Kevin
On 12/01/2026 10:03, Kevin Brodsky wrote: > On 12/01/2026 10:34, Ryan Roberts wrote: >>> - if (!check_vmflag_pfnmap(self->addr1)) >>> - SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file); >> I wonder if we still want this check per-fd, but upgraded to a fail? > > It seems very unlikely to fail though, I can only think of the > underlying file having changed in between but I really wouldn't expect > that for such special files. I was trying not to duplicate the checks > too much. Fair enough. No strong opinion from me. I just thought that given this is a test that claims to be checking the behaviour of pfnmaps, then we ought to be pretty sure we have a pfnmap. > > - Kevin
© 2016 - 2026 Red Hat, Inc.