Re: [PATCH] v4l2-compliance: detect no-mmu systems

From: Hans Verkuil <hidden>
Date: 2021-12-02 11:10:53

Hi Dillon,

On 27/11/2021 12:13, Dillon Min wrote:

Hi Hans,

On Thu, 25 Nov 2021 at 22:12, Hans Verkuil [off-list ref] wrote:

quoted

On 25/11/2021 14:33, Dillon Min wrote:

quoted

Hi Hans,

On Thu, 25 Nov 2021 at 21:14, Hans Verkuil [off-list ref] wrote:

quoted

Check if the OS has an MMU. If not, then skip tests that only work for
systems that have an MMU.

The safest and most generic method I found is the FIONBIO ioctl that is
available for any file descriptor and is a write-only ioctl, so no memory
will be accidentally written. On a MMU system this will return EFAULT, and
on a ucLinux system this will return 0.

Signed-off-by: Hans Verkuil <redacted>
---
Dillon, the original RFC patch (1) I posted to fix this in the kernel is not
the correct method. See:

https://stackoverflow.com/questions/24755103/how-to-handle-null-pointer-argument-to-ioctl-system-call

Thanks for the detailed information.

quoted

So instead I try to detect if there is an MMU in v4l2-compliance and, if not,
just skip those tests that require an MMU.

Can you test this patch?

Sure, I'll test it then update the result.

Note that v4l2-compliance should say at the top that it detects a no-MMU system:

quoted

@@ -152,8 +153,21 @@ static struct option long_options[] = {

 static void print_sha()
 {
+       int fd = open("/dev/null", O_RDONLY);
+
+       if (fd >= 0) {
+               // FIONBIO is a write-only ioctl that takes an int argument that
+               // enables (!= 0) or disables (== 0) nonblocking mode of a fd.
+               //
+               // Passing a nullptr should return EFAULT on MMU capable machines,
+               // and it works if there is no MMU.
+               has_mmu = ioctl(fd, FIONBIO, nullptr);
+               close(fd);
+       }
        printf("v4l2-compliance %s%s, ", PACKAGE_VERSION, STRING(GIT_COMMIT_CNT));
-       printf("%zd bits, %zd-bit time_t\n", sizeof(void *) * 8, sizeof(time_t) * 8);
+       printf("%zd bits, %zd-bit time_t%s\n",
+              sizeof(void *) * 8, sizeof(time_t) * 8,
+              has_mmu ? "" : ", has no MMU");

                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Please verify that it actually does that. I hope it does :-)

Yes, it works.

Thank you for testing this. I merged this patch.

~ # v4l2-compliance -f -d /dev/video0
v4l2-compliance 1.23.0-4896, 32 bits, 32-bit time_t, has no MMU
v4l2-compliance SHA: 7858281c1cd1 2021-11-27 09:43:32

Compliance test for stm-dma2d device /dev/video0:

I added some extra changes for v4l-utils to make the configure step
working on no-mmu platform, just share here , my change just make the
compiler happy, i guess you have a more correct way here: )

Can you post this as a separate patch to the mailinglist? That makes it
easier to handle.

Note that fork can't be converted to vfork in v4l2-test-controls.cpp, so
instead it needs to know if fork is available and put the test under a
#ifdef HAVE_FORK or something like that.

Regards,

	Hans

quoted hunk ↗ jump to hunk

- add --without-argp options for configure.

diff --git a/configure.ac b/configure.ac
index 0529898..3e87b5a 100644
--- a/configure.ac
+++ b/configure.ac

@@ -330,14 +330,23 @@ dl_saved_libs=$LIBS
   AC_SUBST([DLOPEN_LIBS])
 LIBS=$dl_saved_libs

-AC_CHECK_HEADER([argp.h],,AC_MSG_ERROR(Cannot continue: argp.h not found))
+AC_ARG_WITH([argp],
+           AS_HELP_STRING([--without-argp], [Do not use argp.h]),
+           [],
+           [with_argp=yes])
 argp_saved_libs=$LIBS
-  AC_SEARCH_LIBS([argp_parse],
-                 [argp],
-                 [test "$ac_cv_search_argp_parse" = "none required"

|| ARGP_LIBS=$ac_cv_search_argp_parse],
-                 [AC_MSG_ERROR([unable to find the argp_parse() function])])
-  AC_SUBST([ARGP_LIBS])
+AS_IF([test "x$with_argp" != xno],
+      [
+               AC_CHECK_HEADER([argp.h],,AC_MSG_ERROR(Cannot
continue: argp.h not found))]
+      AC_SEARCH_LIBS([argp_parse],
+                    [argp],
+                    [test "$ac_cv_search_argp_parse" = "none
required" || ARGP_LIBS=$ac_cv_search_argp_parse],
+                    [AC_MSG_ERROR([unable to find the argp_parse() function])])
+      AC_SUBST([ARGP_LIBS])
+      ],
+      )
 LIBS=$argp_saved_libs
+AM_CONDITIONAL([HAVE_ARGP], [test "x$with_argp" != xno])

 AC_CHECK_FUNCS([fork],
AC_DEFINE([HAVE_LIBV4LCONVERT_HELPERS],[1],[whether to use
libv4lconvert helpers]))
 AM_CONDITIONAL([HAVE_LIBV4LCONVERT_HELPERS], [test x$ac_cv_func_fork = xyes])

@@ -561,7 +570,7 @@ AM_CONDITIONAL([WITH_GCONV],        [test

x$enable_gconv = xyes -a x$enable_shar
 AM_CONDITIONAL([WITH_V4L2_CTL_LIBV4L], [test
x${enable_v4l2_ctl_libv4l} != xno])
 AM_CONDITIONAL([WITH_V4L2_CTL_STREAM_TO], [test
x${enable_v4l2_ctl_stream_to} != xno])
 AM_CONDITIONAL([WITH_V4L2_CTL_32], [test x${enable_v4l2_ctl_32} = xyes])
-AM_CONDITIONAL([WITH_V4L2_COMPLIANCE], [test x$ac_cv_func_fork = xyes])
+AM_CONDITIONAL([WITH_V4L2_COMPLIANCE], [test x$ac_cv_func_fork = xno])
 AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_LIBV4L], [test x$ac_cv_func_fork
= xyes -a x${enable_v4l2_compliance_libv4l} != xno])
 AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_32], [test x$ac_cv_func_fork =
xyes -a x${enable_v4l2_compliance_32} = xyes])
 PKG_CHECK_MODULES([LIBBPF], [libbpf], [bpf_pc=yes], [bpf_pc=no])

diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am
index 5771ee4..8197e6d 100644
--- a/contrib/test/Makefile.am
+++ b/contrib/test/Makefile.am

@@ -2,9 +2,7 @@ noinst_PROGRAMS = \
        ioctl-test              \
        sliced-vbi-test         \
        sliced-vbi-detect       \
-       v4l2grab                \
        driver-test             \
-       mc_nextgen_test         \
        stress-buffer           \
        capture-example

@@ -13,8 +11,15 @@ noinst_PROGRAMS += pixfmt-test
 endif

 if HAVE_GLU
+if HAVE_ARGP
 noinst_PROGRAMS += v4l2gl
 endif
+endif
+
+if HAVE_ARGP
+noinst_PROGRAMS += mc_nextgen_test.c \
+                  v4l2grab
+endif

- change fork to vfork

diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp

b/utils/v4l2-compliance/v4l2-test-controls.cpp
index 8731c9e..68b2c92 100644

--- a/utils/v4l2-compliance/v4l2-test-controls.cpp
+++ b/utils/v4l2-compliance/v4l2-test-controls.cpp

@@ -978,7 +978,7 @@ int testVividDisconnect(struct node *node)
        // This can only be tested with the vivid driver that enabled
        // the DISCONNECT control.

-       pid_t pid = fork();
+       pid_t pid = vfork();
        if (pid == 0) {
                struct timeval tv = { 5, 0 };
                fd_set fds;

@@ -1010,7 +1010,7 @@ int testVividDisconnect(struct node *node)

        node->reopen();

-       pid = fork();
+       pid = vfork();
        if (pid == 0) {
                struct epoll_event ep_ev;
                int epollfd = epoll_create1(0);

Best Regards
Dillon

quoted

Regards,

        Hans

quoted

        if (strlen(STRING(GIT_SHA)))
                printf("v4l2-compliance SHA: %s %s\n",
                       STRING(GIT_SHA), STRING(GIT_COMMIT_DATE));

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help