All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] v4l2-compliance: detect no-mmu systems
@ 2021-11-25 13:14 Hans Verkuil
  2021-11-25 13:33 ` Dillon Min
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2021-11-25 13:14 UTC (permalink / raw)
  To: Linux Media Mailing List, Dillon Min

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@xs4all.nl>
---
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

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?

Thanks!

	Hans

1: https://patchwork.linuxtv.org/project/linux-media/patch/3acd9ee4-5a58-6ed4-17fe-61596a5252b8@xs4all.nl/
---
diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
index 0eeabb2d..c53a55ba 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -88,6 +88,7 @@ bool is_vivid;
 bool is_uvcvideo;
 int media_fd = -1;
 unsigned warnings;
+bool has_mmu = true;

 static unsigned color_component;
 static unsigned color_skip;
@@ -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");
 	if (strlen(STRING(GIT_SHA)))
 		printf("v4l2-compliance SHA: %s %s\n",
 		       STRING(GIT_SHA), STRING(GIT_COMMIT_DATE));
@@ -623,9 +637,9 @@ static int testCap(struct node *node)
 		V4L2_CAP_VIDEO_M2M;

 	memset(&vcap, 0xff, sizeof(vcap));
-	// Must always be there
-	fail_on_test(doioctl(node, VIDIOC_QUERYCAP, nullptr) != EFAULT);
 	fail_on_test(doioctl(node, VIDIOC_QUERYCAP, &vcap));
+	if (has_mmu)
+		fail_on_test(doioctl(node, VIDIOC_QUERYCAP, nullptr) != EFAULT);
 	fail_on_test(check_ustring(vcap.driver, sizeof(vcap.driver)));
 	fail_on_test(check_ustring(vcap.card, sizeof(vcap.card)));
 	fail_on_test(check_ustring(vcap.bus_info, sizeof(vcap.bus_info)));
@@ -988,11 +1002,10 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
 	}

 	if (driver.empty())
-		printf("Compliance test for device %s%s:\n\n",
-		       node.device, node.g_direct() ? "" : " (using libv4l2)");
+		printf("Compliance test for device ");
 	else
-		printf("Compliance test for %s device %s%s:\n\n",
-		       driver.c_str(), node.device, node.g_direct() ? "" : " (using libv4l2)");
+		printf("Compliance test for %s device ", driver.c_str());
+	printf("%s%s:\n\n", node.device, node.g_direct() ? "" : " (using libv4l2)");

 	if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VBI_CAPTURE |
 			 V4L2_CAP_VIDEO_CAPTURE_MPLANE | V4L2_CAP_SLICED_VBI_CAPTURE |
diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
index e73ebdd3..7255161f 100644
--- a/utils/v4l2-compliance/v4l2-compliance.h
+++ b/utils/v4l2-compliance/v4l2-compliance.h
@@ -58,6 +58,7 @@ extern bool is_uvcvideo; // We're testing the uvc driver
 extern int kernel_version;
 extern int media_fd;
 extern unsigned warnings;
+extern bool has_mmu;

 enum poll_mode {
 	POLL_MODE_NONE,
diff --git a/utils/v4l2-compliance/v4l2-test-io-config.cpp b/utils/v4l2-compliance/v4l2-test-io-config.cpp
index 6f2a9ba9..dcab40b8 100644
--- a/utils/v4l2-compliance/v4l2-test-io-config.cpp
+++ b/utils/v4l2-compliance/v4l2-test-io-config.cpp
@@ -577,6 +577,8 @@ static int checkEdid(struct node *node, unsigned pad, bool is_input)
 		fail_on_test(edid.blocks == 0 || edid.blocks >= 256);
 		fail_on_test(edid.pad != pad);
 	}
+	if (!has_mmu)
+		return 0;
 	edid.blocks = 1;
 	edid.pad = pad;
 	edid.edid = nullptr;
diff --git a/utils/v4l2-compliance/v4l2-test-media.cpp b/utils/v4l2-compliance/v4l2-test-media.cpp
index ef2982c0..28af0d83 100644
--- a/utils/v4l2-compliance/v4l2-test-media.cpp
+++ b/utils/v4l2-compliance/v4l2-test-media.cpp
@@ -32,8 +32,9 @@ int testMediaDeviceInfo(struct node *node)
 	struct media_device_info mdinfo;

 	memset(&mdinfo, 0xff, sizeof(mdinfo));
-	fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, nullptr) != EFAULT);
 	fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, &mdinfo));
+	if (has_mmu)
+		fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, nullptr) != EFAULT);
 	fail_on_test(check_0(mdinfo.reserved, sizeof(mdinfo.reserved)));
 	fail_on_test(check_string(mdinfo.driver, sizeof(mdinfo.driver)));
 	fail_on_test(mdinfo.model[0] && check_string(mdinfo.model, sizeof(mdinfo.model)));
@@ -127,21 +128,23 @@ int testMediaTopology(struct node *node)
 	fail_on_test(topology.reserved2);
 	fail_on_test(topology.reserved3);
 	fail_on_test(topology.reserved4);
-	topology.ptr_entities = 4;
-	fail_on_test(doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
-	topology.ptr_entities = 0;
-	topology.ptr_interfaces = 4;
-	fail_on_test(topology.num_interfaces &&
-		     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
-	topology.ptr_interfaces = 0;
-	topology.ptr_pads = 4;
-	fail_on_test(topology.num_pads &&
-		     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
-	topology.ptr_pads = 0;
-	topology.ptr_links = 4;
-	fail_on_test(topology.num_links &&
-		     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
-	topology.ptr_links = 0;
+	if (has_mmu) {
+		topology.ptr_entities = 4;
+		fail_on_test(doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
+		topology.ptr_entities = 0;
+		topology.ptr_interfaces = 4;
+		fail_on_test(topology.num_interfaces &&
+			     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
+		topology.ptr_interfaces = 0;
+		topology.ptr_pads = 4;
+		fail_on_test(topology.num_pads &&
+			     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
+		topology.ptr_pads = 0;
+		topology.ptr_links = 4;
+		fail_on_test(topology.num_links &&
+			     doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
+		topology.ptr_links = 0;
+	}
 	v2_ents = new media_v2_entity[topology.num_entities];
 	memset(v2_ents, 0xff, topology.num_entities * sizeof(*v2_ents));
 	topology.ptr_entities = (uintptr_t)v2_ents;
@@ -394,12 +397,14 @@ int testMediaEnum(struct node *node)
 		fail_on_test(links.entity != ent.id);
 		fail_on_test(links.pads);
 		fail_on_test(links.links);
-		links.pads = (struct media_pad_desc *)4;
-		fail_on_test(ent.pads && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
-		links.pads = nullptr;
-		links.links = (struct media_link_desc *)4;
-		fail_on_test(ent.links && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
-		links.links = nullptr;
+		if (has_mmu) {
+			links.pads = (struct media_pad_desc *)4;
+			fail_on_test(ent.pads && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
+			links.pads = nullptr;
+			links.links = (struct media_link_desc *)4;
+			fail_on_test(ent.links && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
+			links.links = nullptr;
+		}
 		links.pads = new media_pad_desc[ent.pads];
 		memset(links.pads, 0xff, ent.pads * sizeof(*links.pads));
 		links.links = new media_link_desc[ent.links];
diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
index 68f97205..f3d85771 100644
--- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
+++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
@@ -33,8 +33,9 @@ int testSubDevCap(struct node *node)

 	memset(&caps, 0xff, sizeof(caps));
 	// Must always be there
-	fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, nullptr) != EFAULT);
 	fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, &caps));
+	if (has_mmu)
+		fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, nullptr) != EFAULT);
 	fail_on_test(check_0(caps.reserved, sizeof(caps.reserved)));
 	fail_on_test((caps.version >> 16) < 5);
 	fail_on_test(caps.capabilities & ~VALID_SUBDEV_CAPS);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] v4l2-compliance: detect no-mmu systems
  2021-11-25 13:14 [PATCH] v4l2-compliance: detect no-mmu systems Hans Verkuil
@ 2021-11-25 13:33 ` Dillon Min
  2021-11-25 14:12   ` Hans Verkuil
  0 siblings, 1 reply; 6+ messages in thread
From: Dillon Min @ 2021-11-25 13:33 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List

Hi Hans,

On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
> ---
> 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.

>
> Thanks!
>
>         Hans
>
> 1: https://patchwork.linuxtv.org/project/linux-media/patch/3acd9ee4-5a58-6ed4-17fe-61596a5252b8@xs4all.nl/
> ---
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index 0eeabb2d..c53a55ba 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -88,6 +88,7 @@ bool is_vivid;
>  bool is_uvcvideo;
>  int media_fd = -1;
>  unsigned warnings;
> +bool has_mmu = true;
>
>  static unsigned color_component;
>  static unsigned color_skip;
> @@ -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");
>         if (strlen(STRING(GIT_SHA)))
>                 printf("v4l2-compliance SHA: %s %s\n",
>                        STRING(GIT_SHA), STRING(GIT_COMMIT_DATE));
> @@ -623,9 +637,9 @@ static int testCap(struct node *node)
>                 V4L2_CAP_VIDEO_M2M;
>
>         memset(&vcap, 0xff, sizeof(vcap));
> -       // Must always be there
> -       fail_on_test(doioctl(node, VIDIOC_QUERYCAP, nullptr) != EFAULT);
>         fail_on_test(doioctl(node, VIDIOC_QUERYCAP, &vcap));
> +       if (has_mmu)
> +               fail_on_test(doioctl(node, VIDIOC_QUERYCAP, nullptr) != EFAULT);
>         fail_on_test(check_ustring(vcap.driver, sizeof(vcap.driver)));
>         fail_on_test(check_ustring(vcap.card, sizeof(vcap.card)));
>         fail_on_test(check_ustring(vcap.bus_info, sizeof(vcap.bus_info)));
> @@ -988,11 +1002,10 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
>         }
>
>         if (driver.empty())
> -               printf("Compliance test for device %s%s:\n\n",
> -                      node.device, node.g_direct() ? "" : " (using libv4l2)");
> +               printf("Compliance test for device ");
>         else
> -               printf("Compliance test for %s device %s%s:\n\n",
> -                      driver.c_str(), node.device, node.g_direct() ? "" : " (using libv4l2)");
> +               printf("Compliance test for %s device ", driver.c_str());
> +       printf("%s%s:\n\n", node.device, node.g_direct() ? "" : " (using libv4l2)");
>
>         if (node.g_caps() & (V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VBI_CAPTURE |
>                          V4L2_CAP_VIDEO_CAPTURE_MPLANE | V4L2_CAP_SLICED_VBI_CAPTURE |
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
> index e73ebdd3..7255161f 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -58,6 +58,7 @@ extern bool is_uvcvideo; // We're testing the uvc driver
>  extern int kernel_version;
>  extern int media_fd;
>  extern unsigned warnings;
> +extern bool has_mmu;
>
>  enum poll_mode {
>         POLL_MODE_NONE,
> diff --git a/utils/v4l2-compliance/v4l2-test-io-config.cpp b/utils/v4l2-compliance/v4l2-test-io-config.cpp
> index 6f2a9ba9..dcab40b8 100644
> --- a/utils/v4l2-compliance/v4l2-test-io-config.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-io-config.cpp
> @@ -577,6 +577,8 @@ static int checkEdid(struct node *node, unsigned pad, bool is_input)
>                 fail_on_test(edid.blocks == 0 || edid.blocks >= 256);
>                 fail_on_test(edid.pad != pad);
>         }
> +       if (!has_mmu)
> +               return 0;
>         edid.blocks = 1;
>         edid.pad = pad;
>         edid.edid = nullptr;
> diff --git a/utils/v4l2-compliance/v4l2-test-media.cpp b/utils/v4l2-compliance/v4l2-test-media.cpp
> index ef2982c0..28af0d83 100644
> --- a/utils/v4l2-compliance/v4l2-test-media.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-media.cpp
> @@ -32,8 +32,9 @@ int testMediaDeviceInfo(struct node *node)
>         struct media_device_info mdinfo;
>
>         memset(&mdinfo, 0xff, sizeof(mdinfo));
> -       fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, nullptr) != EFAULT);
>         fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, &mdinfo));
> +       if (has_mmu)
> +               fail_on_test(doioctl(node, MEDIA_IOC_DEVICE_INFO, nullptr) != EFAULT);
>         fail_on_test(check_0(mdinfo.reserved, sizeof(mdinfo.reserved)));
>         fail_on_test(check_string(mdinfo.driver, sizeof(mdinfo.driver)));
>         fail_on_test(mdinfo.model[0] && check_string(mdinfo.model, sizeof(mdinfo.model)));
> @@ -127,21 +128,23 @@ int testMediaTopology(struct node *node)
>         fail_on_test(topology.reserved2);
>         fail_on_test(topology.reserved3);
>         fail_on_test(topology.reserved4);
> -       topology.ptr_entities = 4;
> -       fail_on_test(doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> -       topology.ptr_entities = 0;
> -       topology.ptr_interfaces = 4;
> -       fail_on_test(topology.num_interfaces &&
> -                    doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> -       topology.ptr_interfaces = 0;
> -       topology.ptr_pads = 4;
> -       fail_on_test(topology.num_pads &&
> -                    doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> -       topology.ptr_pads = 0;
> -       topology.ptr_links = 4;
> -       fail_on_test(topology.num_links &&
> -                    doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> -       topology.ptr_links = 0;
> +       if (has_mmu) {
> +               topology.ptr_entities = 4;
> +               fail_on_test(doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> +               topology.ptr_entities = 0;
> +               topology.ptr_interfaces = 4;
> +               fail_on_test(topology.num_interfaces &&
> +                            doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> +               topology.ptr_interfaces = 0;
> +               topology.ptr_pads = 4;
> +               fail_on_test(topology.num_pads &&
> +                            doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> +               topology.ptr_pads = 0;
> +               topology.ptr_links = 4;
> +               fail_on_test(topology.num_links &&
> +                            doioctl(node, MEDIA_IOC_G_TOPOLOGY, &topology) != EFAULT);
> +               topology.ptr_links = 0;
> +       }
>         v2_ents = new media_v2_entity[topology.num_entities];
>         memset(v2_ents, 0xff, topology.num_entities * sizeof(*v2_ents));
>         topology.ptr_entities = (uintptr_t)v2_ents;
> @@ -394,12 +397,14 @@ int testMediaEnum(struct node *node)
>                 fail_on_test(links.entity != ent.id);
>                 fail_on_test(links.pads);
>                 fail_on_test(links.links);
> -               links.pads = (struct media_pad_desc *)4;
> -               fail_on_test(ent.pads && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
> -               links.pads = nullptr;
> -               links.links = (struct media_link_desc *)4;
> -               fail_on_test(ent.links && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
> -               links.links = nullptr;
> +               if (has_mmu) {
> +                       links.pads = (struct media_pad_desc *)4;
> +                       fail_on_test(ent.pads && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
> +                       links.pads = nullptr;
> +                       links.links = (struct media_link_desc *)4;
> +                       fail_on_test(ent.links && doioctl(node, MEDIA_IOC_ENUM_LINKS, &links) != EFAULT);
> +                       links.links = nullptr;
> +               }
>                 links.pads = new media_pad_desc[ent.pads];
>                 memset(links.pads, 0xff, ent.pads * sizeof(*links.pads));
>                 links.links = new media_link_desc[ent.links];
> diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> index 68f97205..f3d85771 100644
> --- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
> @@ -33,8 +33,9 @@ int testSubDevCap(struct node *node)
>
>         memset(&caps, 0xff, sizeof(caps));
>         // Must always be there
> -       fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, nullptr) != EFAULT);
>         fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, &caps));
> +       if (has_mmu)
> +               fail_on_test(doioctl(node, VIDIOC_SUBDEV_QUERYCAP, nullptr) != EFAULT);
>         fail_on_test(check_0(caps.reserved, sizeof(caps.reserved)));
>         fail_on_test((caps.version >> 16) < 5);
>         fail_on_test(caps.capabilities & ~VALID_SUBDEV_CAPS);

Best Regards
Dillon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v4l2-compliance: detect no-mmu systems
  2021-11-25 13:33 ` Dillon Min
@ 2021-11-25 14:12   ` Hans Verkuil
  2021-11-27 11:13     ` Dillon Min
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2021-11-25 14:12 UTC (permalink / raw)
  To: Dillon Min; +Cc: Linux Media Mailing List

On 25/11/2021 14:33, Dillon Min wrote:
> Hi Hans,
> 
> On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
>> ---
>> 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 :-)

Regards,

	Hans

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v4l2-compliance: detect no-mmu systems
  2021-11-25 14:12   ` Hans Verkuil
@ 2021-11-27 11:13     ` Dillon Min
  2021-12-02 11:10       ` Hans Verkuil
  0 siblings, 1 reply; 6+ messages in thread
From: Dillon Min @ 2021-11-27 11:13 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List

Hi Hans,

On Thu, 25 Nov 2021 at 22:12, Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 25/11/2021 14:33, Dillon Min wrote:
> > Hi Hans,
> >
> > On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
> >> ---
> >> 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.

~ # 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: )

- 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));

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] v4l2-compliance: detect no-mmu systems
  2021-11-27 11:13     ` Dillon Min
@ 2021-12-02 11:10       ` Hans Verkuil
  2021-12-02 14:01         ` Dillon Min
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2021-12-02 11:10 UTC (permalink / raw)
  To: Dillon Min; +Cc: Linux Media Mailing List

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@xs4all.nl> wrote:
>>
>> On 25/11/2021 14:33, Dillon Min wrote:
>>> Hi Hans,
>>>
>>> On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
>>>> ---
>>>> 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.

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

> 
> - 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));


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v4l2-compliance: detect no-mmu systems
  2021-12-02 11:10       ` Hans Verkuil
@ 2021-12-02 14:01         ` Dillon Min
  0 siblings, 0 replies; 6+ messages in thread
From: Dillon Min @ 2021-12-02 14:01 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List

Hi Hans

On Thu, 2 Dec 2021 at 19:10, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl> wrote:
> >>
> >> On 25/11/2021 14:33, Dillon Min wrote:
> >>> Hi Hans,
> >>>
> >>> On Thu, 25 Nov 2021 at 21:14, Hans Verkuil <hverkuil-cisco@xs4all.nl> 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@xs4all.nl>
> >>>> ---
> >>>> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-12-02 14:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 13:14 [PATCH] v4l2-compliance: detect no-mmu systems Hans Verkuil
2021-11-25 13:33 ` Dillon Min
2021-11-25 14:12   ` Hans Verkuil
2021-11-27 11:13     ` Dillon Min
2021-12-02 11:10       ` Hans Verkuil
2021-12-02 14:01         ` Dillon Min

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.