Hi Greg,
On Tue, Oct 21, 2025 at 9:41 AM Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Sep 10, 2025 at 07:00:00PM +0200, Gabriele Paoloni wrote:
From: Alessandro Carminati acarmina@redhat.com
This patch introduces a new series of tests for devmem. Test cases are mapped against the tested Function's expectations defined in /drivers/char/mem.c.
Cool, but:
Signed-off-by: Alessandro Carminati acarmina@redhat.com
tools/testing/selftests/Makefile | 1 + tools/testing/selftests/devmem/Makefile | 13 + tools/testing/selftests/devmem/debug.c | 25 + tools/testing/selftests/devmem/debug.h | 14 + tools/testing/selftests/devmem/devmem.c | 200 ++++++++ tools/testing/selftests/devmem/ram_map.c | 250 ++++++++++ tools/testing/selftests/devmem/ram_map.h | 38 ++ tools/testing/selftests/devmem/secret.c | 46 ++ tools/testing/selftests/devmem/secret.h | 13 + tools/testing/selftests/devmem/tests.c | 569 +++++++++++++++++++++++ tools/testing/selftests/devmem/tests.h | 45 ++ tools/testing/selftests/devmem/utils.c | 379 +++++++++++++++ tools/testing/selftests/devmem/utils.h | 119 +++++
That's a lot of files for a "simple" test. Doesn't LTP have tests for this api already? Why not use that here instead?
Indeed, at first glance the test may look simple. However, despite the high-level concept being straightforward, there are several corner cases and peculiar aspects that needed to be covered, that’s why the test grew larger than expected. For this RFC, my intent was mainly to share the full picture of what I had in mind. I fully agree that it can be refined and streamlined in the next iterations. When I started working on this, I wasn’t aware of any existing LTP tests for this interface. After your comment, I double-checked, but I couldn’t find any relevant coverage. As far as I can tell, neither LTP nor the upstream kernel selftests (kselftest or KUnit) currently include tests specifically targeting /dev/mem.
Also, this is userspace testing, not kunit testing, right, is that intentional? You are documenting internal apis and then writing userspace tests for those apis, which feels a bit odd.
Yes, that’s intentional. The main reason is that all the interactions happen from userspace, and the internal helper functions aren’t meant to be consumed by other kernel components. So I chose a kselftest-style approach. That said, this is still an RFC, I’m happy to adapt it based on community feedback and direction.
Also /dev/mem should not be used on "modern" systems, so how was this tested?
That’s indeed one of the nuances I mentioned earlier. Access to /dev/mem is often restricted for security reasons, so the test is designed to detect and adapt to the environment, only performing operations that are actually allowed.
+// SPDX-License-Identifier: GPL-2.0+
Are you _sure_ you want GPLv2+? I have to ask, sorry.
Yes, I used GPLv2+ deliberately, but I’m open to changing it if the consensus is that GPLv2-only is more appropriate here.
+/*
- devmem test debug.c
- Copyright (C) 2025 Red Hat, Inc. All Rights Reserved.
- Written by Alessandro Carminati (acarmina@redhat.com)
- */
+#include <stdio.h> +#include <stdarg.h>
+#define DEBUG_FLAG 0 +int pdebug = DEBUG_FLAG;
That's a funny define that is never used elsewhere. I'm guessing this was cut/pasted from some other userspace code somewhere?
It wasn’t copied, but I agree it looks unnecessary in its current form. I can remove or rework it in the next version.
+void deb_printf(const char *fmt, ...)
Who is "deb"? You have more letters, always use them :)
Also, why debugging for just this one set of tests? Don't kselftests already have debugging logic? if not, why is this unique to require it?
And am I missing something, or does this new tool not tie into the kselftest framework properly? I see lots of printing to output, but not in the proper test framework format, am I just missing that somewhere?
You’re right, this was a small custom debug I left in place, mainly because I needed a way to include additional information that isn’t normally part of the standard selftest output, such as references to specific requirements in the code. My intent was to make it easier to correlate test results with those requirements. That said, I fully agree that the implementation isn’t aligned with the kselftest framework style, and I’ll look into integrating this need in a more standardized and consistent way in the next iteration.
thanks,
greg k-h
Thanks a lot for the detailed feedback, it’s very helpful. I’ll revise the test accordingly for the next round.
Best regards, Alessandro
-- --- 172