Hi Hans On Thu, 2 Dec 2021 at 19:10, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > > Hi Dillon, > > On 27/11/2021 12:13, Dillon Min wrote: > > Hi Hans, > > > > On Thu, 25 Nov 2021 at 22:12, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > >> > >> On 25/11/2021 14:33, Dillon Min wrote: > >>> Hi Hans, > >>> > >>> On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: > >>>> > >>>> 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 <hverkuil-cisco@xxxxxxxxx> > >>>> --- > >>>> 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. > >>> > >>>> > >>>> 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: > >> > >>>> @@ -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. Ok, might be some days later. > > 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. Yes, reasonable. thanks. > > Regards, > > Hans > > > > > - 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 > > > >> > >> Regards, > >> > >> Hans > >> > >>>> if (strlen(STRING(GIT_SHA))) > >>>> printf("v4l2-compliance SHA: %s %s\n", > >>>> STRING(GIT_SHA), STRING(GIT_COMMIT_DATE)); > Best Regards. Dillon
