lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 19 May 2022 20:00:09 +0200
From:   Michael Rodin <mrodin@...adit-jv.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Niklas Söderlund <niklas.soderlund@...natech.se>,
        <linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-renesas-soc@...r.kernel.org>
CC:     Michael Rodin <mrodin@...adit-jv.com>, <michael@...in.online>,
        <erosca@...adit-jv.com>
Subject: [PATCH 3/3] rcar-vin: handle transfer errors from subdevices and stop streaming if required

When a subdevice sends a transfer error event during streaming and we can
not capture new frames, then we know for sure that this is an unrecoverable
failure and not just a temporary glitch. In this case we can not ignore the
transfer error any more and have to notify userspace. In response to the
transfer error event userspace can try to restart streaming and hope that
it works again.

This patch is based on the patch [1] from Niklas Söderlund, however it adds
more logic to check whether the VIN hardware module is actually affected by
the transfer errors reported by the usptream device. For this it takes some
ideas from the imx driver where EOF interrupts are monitored by the
eof_timeout_timer added by commit 4a34ec8e470c ("[media] media: imx: Add
CSI subdev driver").

[1] https://lore.kernel.org/linux-renesas-soc/20211108160220.767586-4-niklas.soderlund+renesas@ragnatech.se/

Signed-off-by: Michael Rodin <mrodin@...adit-jv.com>
---
 drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 34 ++++++++++++++++++++++
 .../media/platform/renesas/rcar-vin/rcar-v4l2.c    | 18 +++++++++++-
 drivers/media/platform/renesas/rcar-vin/rcar-vin.h |  7 +++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
index 2272f1c..596a367 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/pm_runtime.h>
+#include <media/v4l2-event.h>
 
 #include <media/videobuf2-dma-contig.h>
 
@@ -1060,6 +1061,9 @@ static irqreturn_t rvin_irq(int irq, void *data)
 		vin_dbg(vin, "Dropping frame %u\n", vin->sequence);
 	}
 
+	cancel_delayed_work(&vin->frame_timeout);
+	schedule_delayed_work(&vin->frame_timeout, msecs_to_jiffies(FRAME_TIMEOUT_MS));
+
 	vin->sequence++;
 
 	/* Prepare for next frame */
@@ -1283,6 +1287,7 @@ int rvin_start_streaming(struct rvin_dev *vin)
 	spin_lock_irqsave(&vin->qlock, flags);
 
 	vin->sequence = 0;
+	vin->xfer_error = false;
 
 	ret = rvin_capture_start(vin);
 	if (ret)
@@ -1290,6 +1295,10 @@ int rvin_start_streaming(struct rvin_dev *vin)
 
 	spin_unlock_irqrestore(&vin->qlock, flags);
 
+	/* We start the frame watchdog only after we have successfully started streaming */
+	if (!ret)
+		schedule_delayed_work(&vin->frame_timeout, msecs_to_jiffies(FRAME_TIMEOUT_MS));
+
 	return ret;
 }
 
@@ -1332,6 +1341,12 @@ void rvin_stop_streaming(struct rvin_dev *vin)
 	}
 
 	vin->state = STOPPING;
+	/*
+	 * Since we are now stopping and don't expect more frames to be captured, make sure that
+	 * there is no pending work for error handling.
+	 */
+	cancel_delayed_work_sync(&vin->frame_timeout);
+	vin->xfer_error = false;
 
 	/* Wait until only scratch buffer is used, max 3 interrupts. */
 	retries = 0;
@@ -1424,6 +1439,23 @@ void rvin_dma_unregister(struct rvin_dev *vin)
 	v4l2_device_unregister(&vin->v4l2_dev);
 }
 
+static void rvin_frame_timeout(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct rvin_dev *vin = container_of(dwork, struct rvin_dev, frame_timeout);
+	struct v4l2_event event = {
+		.type = V4L2_EVENT_XFER_ERROR,
+	};
+
+	vin_dbg(vin, "Frame timeout!\n");
+
+	if (!vin->xfer_error)
+		return;
+	vin_err(vin, "Unrecoverable transfer error detected, stopping streaming\n");
+	vb2_queue_error(&vin->queue);
+	v4l2_event_queue(&vin->vdev, &event);
+}
+
 int rvin_dma_register(struct rvin_dev *vin, int irq)
 {
 	struct vb2_queue *q = &vin->queue;
@@ -1470,6 +1502,8 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
 		goto error;
 	}
 
+	INIT_DELAYED_WORK(&vin->frame_timeout, rvin_frame_timeout);
+
 	return 0;
 error:
 	rvin_dma_unregister(vin);
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
index 2e2aa9d..bd7f6fe2 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
@@ -648,6 +648,8 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
 	switch (sub->type) {
 	case V4L2_EVENT_SOURCE_CHANGE:
 		return v4l2_event_subscribe(fh, sub, 4, NULL);
+	case V4L2_EVENT_XFER_ERROR:
+		return v4l2_event_subscribe(fh, sub, 1, NULL);
 	}
 	return v4l2_ctrl_subscribe_event(fh, sub);
 }
@@ -1000,9 +1002,23 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
 static void rvin_notify_video_device(struct rvin_dev *vin,
 				     unsigned int notification, void *arg)
 {
+	const struct v4l2_event *event;
+
 	switch (notification) {
 	case V4L2_DEVICE_NOTIFY_EVENT:
-		v4l2_event_queue(&vin->vdev, arg);
+		event = arg;
+
+		switch (event->type) {
+		case V4L2_EVENT_XFER_ERROR:
+			if (vin->state != STOPPED && vin->state != STOPPING) {
+				vin_dbg(vin, "Subdevice signaled transfer error.\n");
+				vin->xfer_error = true;
+			}
+			break;
+		default:
+			break;
+		}
+
 		break;
 	default:
 		break;
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index 1f94589..4726a69 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -31,6 +31,9 @@
 /* Max number on VIN instances that can be in a system */
 #define RCAR_VIN_NUM 32
 
+/* maximum time we wait before signalling an error to userspace */
+#define FRAME_TIMEOUT_MS 1000
+
 struct rvin_group;
 
 enum model_id {
@@ -207,6 +210,8 @@ struct rvin_info {
  * @std:		active video standard of the video source
  *
  * @alpha:		Alpha component to fill in for supported pixel formats
+ * @xfer_error:		Indicates if any transfer errors occurred in the current streaming session.
+ * @frame_timeout:	Watchdog for monitoring regular capturing of frames in rvin_irq.
  */
 struct rvin_dev {
 	struct device *dev;
@@ -251,6 +256,8 @@ struct rvin_dev {
 	v4l2_std_id std;
 
 	unsigned int alpha;
+	bool xfer_error;
+	struct delayed_work frame_timeout;
 };
 
 #define vin_to_source(vin)		((vin)->parallel.subdev)
-- 
2.7.4

Powered by blists - more mailing lists