Add to the v4l2 framework an helper function to support drivers
when validating a buffer of extensible parameters.
Introduce new types in include/media/v4l2-params.h that drivers shall
use in order to comply with the v4l2-params validation procedure, and
add a helper functions to v4l2-params.c to perform block and buffer
validation.
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
MAINTAINERS | 2 +
drivers/media/v4l2-core/Makefile | 3 +-
drivers/media/v4l2-core/v4l2-params.c | 126 ++++++++++++++++++++++++++
include/media/v4l2-params.h | 164 ++++++++++++++++++++++++++++++++++
4 files changed, 294 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 91df04e5d9022ccf2aea4445247369a8b86a4264..008f984c0769691f6ddec8d8f0f461fde056ddb3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26385,6 +26385,8 @@ M: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
L: linux-media@vger.kernel.org
S: Maintained
F: Documentation/userspace-api/media/v4l/extensible-parameters.rst
+F: drivers/media/v4l2-core/v4l2-params.c
+F: include/media/v4l2-params.h
F: include/uapi/linux/media/v4l2-extensible-params.h
VF610 NAND DRIVER
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 2177b9d63a8ffc1127c5a70118249a2ff63cd759..323330dd359f95c1ae3d0c35bd6fcb8291a33a07 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -11,7 +11,8 @@ tuner-objs := tuner-core.o
videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
v4l2-event.o v4l2-subdev.o v4l2-common.o \
v4l2-ctrls-core.o v4l2-ctrls-api.o \
- v4l2-ctrls-request.o v4l2-ctrls-defs.o
+ v4l2-ctrls-request.o v4l2-ctrls-defs.o \
+ v4l2-params.o
# Please keep it alphabetically sorted by Kconfig name
# (e. g. LC_ALL=C sort Makefile)
diff --git a/drivers/media/v4l2-core/v4l2-params.c b/drivers/media/v4l2-core/v4l2-params.c
new file mode 100644
index 0000000000000000000000000000000000000000..69fc7fabce9dac3fca5a6b783b86a3b0dfc1d803
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-params.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Video4Linux2 extensible parameters helpers
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Author: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <media/videobuf2-core.h>
+
+#include <media/v4l2-params.h>
+
+int v4l2_params_buffer_validate(struct device *dev, struct vb2_buffer *vb,
+ size_t max_size,
+ v4l2_params_validate_buffer buffer_validate)
+{
+ size_t header_size = offsetof(struct v4l2_params_buffer, data);
+ struct v4l2_params_buffer *buffer = vb2_plane_vaddr(vb, 0);
+ size_t payload_size = vb2_get_plane_payload(vb, 0);
+ size_t buffer_size;
+ int ret;
+
+ /* Payload size can't be greater than the destination buffer size */
+ if (payload_size > max_size) {
+ dev_dbg(dev, "Payload size is too large: %zu\n", payload_size);
+ return -EINVAL;
+ }
+
+ /* Payload size can't be smaller than the header size */
+ if (payload_size < header_size) {
+ dev_dbg(dev, "Payload size is too small: %zu\n", payload_size);
+ return -EINVAL;
+ }
+
+ /* Validate the size reported in the parameter buffer header */
+ buffer_size = header_size + buffer->data_size;
+ if (buffer_size != payload_size) {
+ dev_dbg(dev, "Data size %zu and payload size %zu are different\n",
+ buffer_size, payload_size);
+ return -EINVAL;
+ }
+
+ /* Driver-specific buffer validation. */
+ if (buffer_validate) {
+ ret = buffer_validate(dev, buffer);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_params_buffer_validate);
+
+int v4l2_params_blocks_validate(struct device *dev,
+ const struct v4l2_params_buffer *buffer,
+ const struct v4l2_params_handler *handlers,
+ size_t num_handlers,
+ v4l2_params_validate_block block_validate)
+{
+ size_t block_offset = 0;
+ size_t buffer_size;
+ int ret;
+
+ /* Walk the list of parameter blocks and validate them. */
+ buffer_size = buffer->data_size;
+ while (buffer_size >= sizeof(struct v4l2_params_block_header)) {
+ const struct v4l2_params_handler *handler;
+ const struct v4l2_params_block_header *block;
+
+ /* Validate block sizes and types against the handlers. */
+ block = (const struct v4l2_params_block_header *)
+ (buffer->data + block_offset);
+
+ if (block->type >= num_handlers) {
+ dev_dbg(dev, "Invalid parameters block type\n");
+ return -EINVAL;
+ }
+
+ if (block->size > buffer_size) {
+ dev_dbg(dev, "Premature end of parameters data\n");
+ return -EINVAL;
+ }
+
+ /* It's invalid to specify both ENABLE and DISABLE. */
+ if ((block->flags & (V4L2_PARAMS_FL_BLOCK_ENABLE |
+ V4L2_PARAMS_FL_BLOCK_DISABLE)) ==
+ (V4L2_PARAMS_FL_BLOCK_ENABLE |
+ V4L2_PARAMS_FL_BLOCK_DISABLE)) {
+ dev_dbg(dev, "Invalid parameters block flags\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Match the block reported size against the handler's expected
+ * one, but allow the block to only contain the header in
+ * case it is going to be disabled.
+ */
+ handler = &handlers[block->type];
+ if (block->size != handler->size &&
+ (!(block->flags & V4L2_PARAMS_FL_BLOCK_DISABLE) ||
+ block->size != sizeof(*block))) {
+ dev_dbg(dev, "Invalid parameters block size\n");
+ return -EINVAL;
+ }
+
+ /* Driver-specific per-block validation. */
+ if (block_validate) {
+ ret = block_validate(dev, block);
+ if (ret)
+ return ret;
+ }
+
+ block_offset += block->size;
+ buffer_size -= block->size;
+ }
+
+ if (buffer_size) {
+ dev_dbg(dev, "Unexpected data after the parameters buffer end\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_params_blocks_validate);
diff --git a/include/media/v4l2-params.h b/include/media/v4l2-params.h
new file mode 100644
index 0000000000000000000000000000000000000000..f42d56a6d6a0383571116345d76ba02fc9ae039d
--- /dev/null
+++ b/include/media/v4l2-params.h
@@ -0,0 +1,164 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Video4Linux2 extensible parameters helpers
+ *
+ * Copyright (C) 2025 Ideas On Board Oy
+ * Author: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+ */
+
+#ifndef V4L2_PARAMS_H_
+#define V4L2_PARAMS_H_
+
+#include <linux/media/v4l2-extensible-params.h>
+
+struct device;
+struct vb2_buffer;
+
+/**
+ * typedef v4l2_params_block_handler - V4L2 extensible format block handler
+ * @arg: pointer the driver-specific argument
+ * @block: the ISP configuration block to handle
+ *
+ * Defines the function signature of the functions that handle an ISP block
+ * configuration.
+ */
+typedef void (*v4l2_params_block_handler)(void *arg,
+ const struct v4l2_params_block_header *block);
+
+/**
+ * struct v4l2_params_handler - V4L2 extensible format handler
+ * @size: the block expected size
+ * @handler: the block handler function
+ * @group: the device-specific group id the block belongs to (optional)
+ * @features: the device-specific features flags (optional)
+ *
+ * The v4l2_params_handler defines the type that driver making use of the
+ * V4L2 extensible parameters shall use to define their own ISP block
+ * handlers.
+ *
+ * Drivers shall prepare a list of handlers, one for each supported ISP block
+ * and correctly populate the structure's field with the expected block @size
+ * (used for validation), a pointer to each block @handler function and an
+ * optional @group and @feature flags, the driver can use to differentiate which
+ * ISP blocks are present on the ISP implementation.
+ *
+ * The @group field is intended to be used as a bitmask of driver-specific
+ * flags to allow the driver to setup certain blocks at different times. As an
+ * example an ISP driver can divide its block handlers in "pre-configure" blocks
+ * and "run-time" blocks and use the @group bitmask to identify the ISP blocks
+ * that have to be pre-configured from the ones that only have to be handled at
+ * run-time. The usage and definition of the @group field is totally
+ * driver-specific.
+ *
+ * The @features flag can instead be used to differentiate between blocks
+ * implemented in different revisions of the ISP design. In example some ISP
+ * blocks might be present on more recent revision than others. Populating the
+ * @features bitmask with the ISP/SoC machine identifier allows the driver to
+ * correctly ignore the blocks not supported on the ISP revision it is running
+ * on. As per the @group bitmask, the usage and definition of the @features
+ * field is totally driver-specific.
+ */
+struct v4l2_params_handler {
+ size_t size;
+ v4l2_params_block_handler handler;
+ unsigned int group;
+ unsigned int features;
+};
+
+/**
+ * typedef v4l2_params_validate_buffer - V4L2 extensible parameters buffer
+ * validation callback
+ * @dev: the driver's device pointer (as passed by the driver to
+ * v4l2_params_buffer_validate())
+ * @buffer: the extensible parameters buffer
+ *
+ * Defines the function prototype for the driver's callback to perform
+ * driver-specific validation on the extensible parameters buffer
+ */
+typedef int (*v4l2_params_validate_buffer)(struct device *dev,
+ const struct v4l2_params_buffer *buffer);
+
+/**
+ * v4l2_params_buffer_validate - Validate a V4L2 extensible parameters buffer
+ * @dev: the driver's device pointer
+ * @vb: the videobuf2 buffer
+ * @max_size: the maximum allowed buffer size
+ * @buffer_validate: callback to the driver-specific buffer validation
+ *
+ * Helper function that performs validation of an extensible parameters buffer.
+ *
+ * The helper is meant to be used by drivers to perform validation of the
+ * extensible parameters buffer size correctness.
+ *
+ * The @vb buffer as received from the vb2 .buf_prepare() operation is checked
+ * against @max_size and its validated to be large enough to accommodate at
+ * least one ISP configuration block. The effective buffer size is compared
+ * with the reported data size to make sure they match.
+ *
+ * If provided, the @buffer_validate callback function is invoked to allow
+ * drivers to perform driver-specific validation (such as checking that the
+ * buffer version is supported).
+ *
+ * Drivers should use this function to validate the buffer size correctness
+ * before performing a copy of the user-provided videobuf2 buffer content into a
+ * kernel-only memory buffer to prevent userspace from modifying the buffer
+ * content after it has been submitted to the driver.
+ *.
+ * Examples of users of this function can be found in
+ * rkisp1_params_prepare_ext_params() and in c3_isp_params_vb2_buf_prepare().
+ */
+int v4l2_params_buffer_validate(struct device *dev, struct vb2_buffer *vb,
+ size_t max_size,
+ v4l2_params_validate_buffer buffer_validate);
+
+/**
+ * typedef v4l2_params_validate_block - V4L2 extensible parameters block
+ * validation callback
+ * @dev: the driver's device pointer (as passed by the driver to
+ * v4l2_params_validate())
+ * @block: the ISP configuration block to validate
+ *
+ * Defines the function prototype for the driver's callback to perform
+ * driver-specific validation on each ISP block.
+ */
+typedef int (*v4l2_params_validate_block)(struct device *dev,
+ const struct v4l2_params_block_header *block);
+
+/**
+ * v4l2_params_blocks_validate - Validate V4L2 extensible parameters ISP
+ * configuration blocks
+ * @dev: the driver's device pointer
+ * @buffer: the extensible parameters configuration buffer
+ * @handlers: the list of block handlers
+ * @num_handlers: the number of block handlers
+ * @block_validate: callback to the driver-specific per-block validation
+ * function
+ *
+ * Helper function that performs validation of the ISP configuration blocks in
+ * an extensible parameters buffer.
+ *
+ * The helper is meant to be used by drivers to perform validation of the
+ * ISP configuration data blocks. For each block in the extensible parameters
+ * buffer, its size and correctness are validated against its associated handler
+ * in the @handlers list. Additionally, if provided, the @block_validate
+ * callback is invoked on each block to allow drivers to perform driver-specific
+ * validation.
+ *
+ * Drivers should use this function to validate the ISP configuration blocks
+ * after having validated the correctness of the vb2 buffer sizes by using the
+ * v4l2_params_buffer_validate() helper first. Once the buffer size has been
+ * validated, drivers should perform a copy of the user-provided buffer into a
+ * kernel-only memory buffer to prevent userspace from modifying the buffer
+ * content after it has been submitted to the driver, and then call this
+ * function to perform per-block validation.
+ *
+ * Examples of users of this function can be found in
+ * rkisp1_params_prepare_ext_params() and in c3_isp_params_vb2_buf_prepare().
+ */
+int v4l2_params_blocks_validate(struct device *dev,
+ const struct v4l2_params_buffer *buffer,
+ const struct v4l2_params_handler *handlers,
+ size_t num_handlers,
+ v4l2_params_validate_block block_validate);
+
+#endif /* V4L2_PARAMS_H_ */
--
2.50.1On Wed, Aug 20, 2025 at 02:58:13PM +0200, Jacopo Mondi wrote:
> Add to the v4l2 framework an helper function to support drivers
s/an helper/a helper/
> when validating a buffer of extensible parameters.
>
> Introduce new types in include/media/v4l2-params.h that drivers shall
As for the UAPI, I'd go straight for v4l2-isp.[ch].
> use in order to comply with the v4l2-params validation procedure, and
> add a helper functions to v4l2-params.c to perform block and buffer
> validation.
>
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> MAINTAINERS | 2 +
> drivers/media/v4l2-core/Makefile | 3 +-
> drivers/media/v4l2-core/v4l2-params.c | 126 ++++++++++++++++++++++++++
> include/media/v4l2-params.h | 164 ++++++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 91df04e5d9022ccf2aea4445247369a8b86a4264..008f984c0769691f6ddec8d8f0f461fde056ddb3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26385,6 +26385,8 @@ M: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> L: linux-media@vger.kernel.org
> S: Maintained
> F: Documentation/userspace-api/media/v4l/extensible-parameters.rst
> +F: drivers/media/v4l2-core/v4l2-params.c
> +F: include/media/v4l2-params.h
> F: include/uapi/linux/media/v4l2-extensible-params.h
>
> VF610 NAND DRIVER
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index 2177b9d63a8ffc1127c5a70118249a2ff63cd759..323330dd359f95c1ae3d0c35bd6fcb8291a33a07 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -11,7 +11,8 @@ tuner-objs := tuner-core.o
> videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> v4l2-event.o v4l2-subdev.o v4l2-common.o \
> v4l2-ctrls-core.o v4l2-ctrls-api.o \
> - v4l2-ctrls-request.o v4l2-ctrls-defs.o
> + v4l2-ctrls-request.o v4l2-ctrls-defs.o \
> + v4l2-params.o
>
> # Please keep it alphabetically sorted by Kconfig name
> # (e. g. LC_ALL=C sort Makefile)
> diff --git a/drivers/media/v4l2-core/v4l2-params.c b/drivers/media/v4l2-core/v4l2-params.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..69fc7fabce9dac3fca5a6b783b86a3b0dfc1d803
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-params.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Video4Linux2 extensible parameters helpers
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Author: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <media/videobuf2-core.h>
> +
The blank line should be before the previous header.
> +#include <media/v4l2-params.h>
> +
> +int v4l2_params_buffer_validate(struct device *dev, struct vb2_buffer *vb,
> + size_t max_size,
> + v4l2_params_validate_buffer buffer_validate)
> +{
> + size_t header_size = offsetof(struct v4l2_params_buffer, data);
> + struct v4l2_params_buffer *buffer = vb2_plane_vaddr(vb, 0);
> + size_t payload_size = vb2_get_plane_payload(vb, 0);
> + size_t buffer_size;
> + int ret;
> +
> + /* Payload size can't be greater than the destination buffer size */
> + if (payload_size > max_size) {
> + dev_dbg(dev, "Payload size is too large: %zu\n", payload_size);
> + return -EINVAL;
> + }
> +
> + /* Payload size can't be smaller than the header size */
> + if (payload_size < header_size) {
> + dev_dbg(dev, "Payload size is too small: %zu\n", payload_size);
> + return -EINVAL;
> + }
> +
> + /* Validate the size reported in the parameter buffer header */
> + buffer_size = header_size + buffer->data_size;
> + if (buffer_size != payload_size) {
> + dev_dbg(dev, "Data size %zu and payload size %zu are different\n",
> + buffer_size, payload_size);
> + return -EINVAL;
> + }
> +
> + /* Driver-specific buffer validation. */
> + if (buffer_validate) {
> + ret = buffer_validate(dev, buffer);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_params_buffer_validate);
> +
> +int v4l2_params_blocks_validate(struct device *dev,
> + const struct v4l2_params_buffer *buffer,
> + const struct v4l2_params_handler *handlers,
> + size_t num_handlers,
> + v4l2_params_validate_block block_validate)
> +{
> + size_t block_offset = 0;
> + size_t buffer_size;
> + int ret;
> +
> + /* Walk the list of parameter blocks and validate them. */
> + buffer_size = buffer->data_size;
> + while (buffer_size >= sizeof(struct v4l2_params_block_header)) {
> + const struct v4l2_params_handler *handler;
> + const struct v4l2_params_block_header *block;
> +
> + /* Validate block sizes and types against the handlers. */
> + block = (const struct v4l2_params_block_header *)
> + (buffer->data + block_offset);
> +
> + if (block->type >= num_handlers) {
> + dev_dbg(dev, "Invalid parameters block type\n");
> + return -EINVAL;
> + }
> +
> + if (block->size > buffer_size) {
> + dev_dbg(dev, "Premature end of parameters data\n");
> + return -EINVAL;
> + }
> +
> + /* It's invalid to specify both ENABLE and DISABLE. */
> + if ((block->flags & (V4L2_PARAMS_FL_BLOCK_ENABLE |
> + V4L2_PARAMS_FL_BLOCK_DISABLE)) ==
> + (V4L2_PARAMS_FL_BLOCK_ENABLE |
> + V4L2_PARAMS_FL_BLOCK_DISABLE)) {
> + dev_dbg(dev, "Invalid parameters block flags\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Match the block reported size against the handler's expected
> + * one, but allow the block to only contain the header in
> + * case it is going to be disabled.
> + */
> + handler = &handlers[block->type];
> + if (block->size != handler->size &&
> + (!(block->flags & V4L2_PARAMS_FL_BLOCK_DISABLE) ||
> + block->size != sizeof(*block))) {
> + dev_dbg(dev, "Invalid parameters block size\n");
> + return -EINVAL;
> + }
> +
> + /* Driver-specific per-block validation. */
> + if (block_validate) {
> + ret = block_validate(dev, block);
> + if (ret)
> + return ret;
> + }
> +
> + block_offset += block->size;
> + buffer_size -= block->size;
> + }
> +
> + if (buffer_size) {
> + dev_dbg(dev, "Unexpected data after the parameters buffer end\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_params_blocks_validate);
> diff --git a/include/media/v4l2-params.h b/include/media/v4l2-params.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..f42d56a6d6a0383571116345d76ba02fc9ae039d
> --- /dev/null
> +++ b/include/media/v4l2-params.h
> @@ -0,0 +1,164 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Video4Linux2 extensible parameters helpers
> + *
> + * Copyright (C) 2025 Ideas On Board Oy
> + * Author: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> + */
> +
> +#ifndef V4L2_PARAMS_H_
> +#define V4L2_PARAMS_H_
> +
> +#include <linux/media/v4l2-extensible-params.h>
> +
> +struct device;
> +struct vb2_buffer;
> +
> +/**
> + * typedef v4l2_params_block_handler - V4L2 extensible format block handler
> + * @arg: pointer the driver-specific argument
> + * @block: the ISP configuration block to handle
> + *
> + * Defines the function signature of the functions that handle an ISP block
> + * configuration.
> + */
> +typedef void (*v4l2_params_block_handler)(void *arg,
> + const struct v4l2_params_block_header *block);
> +
> +/**
> + * struct v4l2_params_handler - V4L2 extensible format handler
> + * @size: the block expected size
> + * @handler: the block handler function
> + * @group: the device-specific group id the block belongs to (optional)
> + * @features: the device-specific features flags (optional)
Only the size is used by the validation helpers. The group and features
fields are used by rkisp1 only. I think we should either generalize the
concepts, or drop them from this structure. The group field, in
particular, should likely be dropped.
> + *
> + * The v4l2_params_handler defines the type that driver making use of the
> + * V4L2 extensible parameters shall use to define their own ISP block
> + * handlers.
> + *
> + * Drivers shall prepare a list of handlers, one for each supported ISP block
> + * and correctly populate the structure's field with the expected block @size
> + * (used for validation), a pointer to each block @handler function and an
> + * optional @group and @feature flags, the driver can use to differentiate which
> + * ISP blocks are present on the ISP implementation.
> + *
> + * The @group field is intended to be used as a bitmask of driver-specific
> + * flags to allow the driver to setup certain blocks at different times. As an
> + * example an ISP driver can divide its block handlers in "pre-configure" blocks
> + * and "run-time" blocks and use the @group bitmask to identify the ISP blocks
> + * that have to be pre-configured from the ones that only have to be handled at
> + * run-time. The usage and definition of the @group field is totally
> + * driver-specific.
> + *
> + * The @features flag can instead be used to differentiate between blocks
> + * implemented in different revisions of the ISP design. In example some ISP
> + * blocks might be present on more recent revision than others. Populating the
> + * @features bitmask with the ISP/SoC machine identifier allows the driver to
> + * correctly ignore the blocks not supported on the ISP revision it is running
> + * on. As per the @group bitmask, the usage and definition of the @features
> + * field is totally driver-specific.
> + */
> +struct v4l2_params_handler {
> + size_t size;
> + v4l2_params_block_handler handler;
> + unsigned int group;
> + unsigned int features;
> +};
> +
> +/**
> + * typedef v4l2_params_validate_buffer - V4L2 extensible parameters buffer
> + * validation callback
> + * @dev: the driver's device pointer (as passed by the driver to
> + * v4l2_params_buffer_validate())
> + * @buffer: the extensible parameters buffer
> + *
> + * Defines the function prototype for the driver's callback to perform
> + * driver-specific validation on the extensible parameters buffer
> + */
> +typedef int (*v4l2_params_validate_buffer)(struct device *dev,
> + const struct v4l2_params_buffer *buffer);
> +
Don't create mid-layers. If the driver needs to perform additional
validation, do so before or after calling v4l2_params_buffer_validate().
> +/**
> + * v4l2_params_buffer_validate - Validate a V4L2 extensible parameters buffer
> + * @dev: the driver's device pointer
> + * @vb: the videobuf2 buffer
> + * @max_size: the maximum allowed buffer size
> + * @buffer_validate: callback to the driver-specific buffer validation
> + *
> + * Helper function that performs validation of an extensible parameters buffer.
> + *
> + * The helper is meant to be used by drivers to perform validation of the
> + * extensible parameters buffer size correctness.
> + *
> + * The @vb buffer as received from the vb2 .buf_prepare() operation is checked
> + * against @max_size and its validated to be large enough to accommodate at
> + * least one ISP configuration block. The effective buffer size is compared
> + * with the reported data size to make sure they match.
> + *
> + * If provided, the @buffer_validate callback function is invoked to allow
> + * drivers to perform driver-specific validation (such as checking that the
> + * buffer version is supported).
> + *
> + * Drivers should use this function to validate the buffer size correctness
> + * before performing a copy of the user-provided videobuf2 buffer content into a
> + * kernel-only memory buffer to prevent userspace from modifying the buffer
> + * content after it has been submitted to the driver.
> + *.
Extra period.
> + * Examples of users of this function can be found in
> + * rkisp1_params_prepare_ext_params() and in c3_isp_params_vb2_buf_prepare().
Drop this, git grep is enough to find users.
> + */
> +int v4l2_params_buffer_validate(struct device *dev, struct vb2_buffer *vb,
> + size_t max_size,
> + v4l2_params_validate_buffer buffer_validate);
> +
> +/**
> + * typedef v4l2_params_validate_block - V4L2 extensible parameters block
> + * validation callback
> + * @dev: the driver's device pointer (as passed by the driver to
> + * v4l2_params_validate())
> + * @block: the ISP configuration block to validate
> + *
> + * Defines the function prototype for the driver's callback to perform
> + * driver-specific validation on each ISP block.
> + */
> +typedef int (*v4l2_params_validate_block)(struct device *dev,
> + const struct v4l2_params_block_header *block);
> +
This isn't used in this series. Drop it and introduce it later if needed
by another driver, I can't review the feature without seeing a user.
Another feature that may be helpful to move to the V4L2 core (in a
separate patch or a separate series) is copying the parameters to an
internal buffer.
> +/**
> + * v4l2_params_blocks_validate - Validate V4L2 extensible parameters ISP
> + * configuration blocks
> + * @dev: the driver's device pointer
> + * @buffer: the extensible parameters configuration buffer
> + * @handlers: the list of block handlers
> + * @num_handlers: the number of block handlers
> + * @block_validate: callback to the driver-specific per-block validation
> + * function
> + *
> + * Helper function that performs validation of the ISP configuration blocks in
> + * an extensible parameters buffer.
> + *
> + * The helper is meant to be used by drivers to perform validation of the
> + * ISP configuration data blocks. For each block in the extensible parameters
> + * buffer, its size and correctness are validated against its associated handler
> + * in the @handlers list. Additionally, if provided, the @block_validate
> + * callback is invoked on each block to allow drivers to perform driver-specific
> + * validation.
> + *
> + * Drivers should use this function to validate the ISP configuration blocks
> + * after having validated the correctness of the vb2 buffer sizes by using the
> + * v4l2_params_buffer_validate() helper first. Once the buffer size has been
> + * validated, drivers should perform a copy of the user-provided buffer into a
> + * kernel-only memory buffer to prevent userspace from modifying the buffer
> + * content after it has been submitted to the driver, and then call this
> + * function to perform per-block validation.
> + *
> + * Examples of users of this function can be found in
> + * rkisp1_params_prepare_ext_params() and in c3_isp_params_vb2_buf_prepare().
Drop this.
> + */
> +int v4l2_params_blocks_validate(struct device *dev,
> + const struct v4l2_params_buffer *buffer,
> + const struct v4l2_params_handler *handlers,
> + size_t num_handlers,
> + v4l2_params_validate_block block_validate);
> +
> +#endif /* V4L2_PARAMS_H_ */
--
Regards,
Laurent Pinchart
Hi Jacopo, kernel test robot noticed the following build errors: [auto build test ERROR on a75b8d198c55e9eb5feb6f6e155496305caba2dc] url: https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-uapi-Introduce-V4L2-extensible-params/20250820-210503 base: a75b8d198c55e9eb5feb6f6e155496305caba2dc patch link: https://lore.kernel.org/r/20250820-extensible-parameters-validation-v4-5-30fe5a99cb1f%40ideasonboard.com patch subject: [PATCH v4 5/8] media: v4l2-core: Introduce v4l2-params.c config: nios2-randconfig-002-20250824 (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/config) compiler: nios2-linux-gcc (GCC) 9.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508240704.AZwGXBaw-lkp@intel.com/ All errors (new ones prefixed by >>): nios2-linux-ld: drivers/media/v4l2-core/v4l2-params.o: in function `v4l2_params_buffer_validate': v4l2-params.c:(.text+0x124): undefined reference to `vb2_plane_vaddr' >> v4l2-params.c:(.text+0x124): relocation truncated to fit: R_NIOS2_CALL26 against `vb2_plane_vaddr' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Question for media maintainers... I'm not sure how I should better handle this one On Sun, Aug 24, 2025 at 07:16:21AM +0800, kernel test robot wrote: > Hi Jacopo, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on a75b8d198c55e9eb5feb6f6e155496305caba2dc] > > url: https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-uapi-Introduce-V4L2-extensible-params/20250820-210503 > base: a75b8d198c55e9eb5feb6f6e155496305caba2dc > patch link: https://lore.kernel.org/r/20250820-extensible-parameters-validation-v4-5-30fe5a99cb1f%40ideasonboard.com > patch subject: [PATCH v4 5/8] media: v4l2-core: Introduce v4l2-params.c > config: nios2-randconfig-002-20250824 (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/config) > compiler: nios2-linux-gcc (GCC) 9.5.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202508240704.AZwGXBaw-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > nios2-linux-ld: drivers/media/v4l2-core/v4l2-params.o: in function `v4l2_params_buffer_validate': > v4l2-params.c:(.text+0x124): undefined reference to `vb2_plane_vaddr' > >> v4l2-params.c:(.text+0x124): relocation truncated to fit: R_NIOS2_CALL26 against `vb2_plane_vaddr' > Clearly v4l2-params.c (now v4l2-isp.c) depends on VIDEOBUF2_CORE Right now v4l2-params.c gets compiled in as part of the videodev-objs Makefile target. IOW is not gated by a KConfig option. To select (or better, depend) on VIDEOBUF2_CORE I should guard v4l2-params.c with a KConfig option so that I can 'depend on VIDEOBUF2_CORE'. Drivers that use the v4l2-params.c helper would then have to 'select V4L2_PARAMS` (or similar). Is this the best approach or are there alternatives I might be missing ? > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki >
On Mon, Sep 15, 2025 at 03:31:58PM +0200, Jacopo Mondi wrote: > Question for media maintainers... > > I'm not sure how I should better handle this one > > On Sun, Aug 24, 2025 at 07:16:21AM +0800, kernel test robot wrote: > > Hi Jacopo, > > > > kernel test robot noticed the following build errors: > > > > [auto build test ERROR on a75b8d198c55e9eb5feb6f6e155496305caba2dc] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Jacopo-Mondi/media-uapi-Introduce-V4L2-extensible-params/20250820-210503 > > base: a75b8d198c55e9eb5feb6f6e155496305caba2dc > > patch link: https://lore.kernel.org/r/20250820-extensible-parameters-validation-v4-5-30fe5a99cb1f%40ideasonboard.com > > patch subject: [PATCH v4 5/8] media: v4l2-core: Introduce v4l2-params.c > > config: nios2-randconfig-002-20250824 (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/config) > > compiler: nios2-linux-gcc (GCC) 9.5.0 > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250824/202508240704.AZwGXBaw-lkp@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202508240704.AZwGXBaw-lkp@intel.com/ > > > > All errors (new ones prefixed by >>): > > > > nios2-linux-ld: drivers/media/v4l2-core/v4l2-params.o: in function `v4l2_params_buffer_validate': > > v4l2-params.c:(.text+0x124): undefined reference to `vb2_plane_vaddr' > > >> v4l2-params.c:(.text+0x124): relocation truncated to fit: R_NIOS2_CALL26 against `vb2_plane_vaddr' > > Clearly v4l2-params.c (now v4l2-isp.c) depends on VIDEOBUF2_CORE > > Right now v4l2-params.c gets compiled in as part of the > videodev-objs Makefile target. IOW is not gated by a KConfig option. > > To select (or better, depend) on VIDEOBUF2_CORE I should guard > v4l2-params.c with a KConfig option so that I can 'depend on > VIDEOBUF2_CORE'. Drivers that use the v4l2-params.c helper would then > have to 'select V4L2_PARAMS` (or similar). Make it V4L2_ISP :-) > Is this the best approach or are there alternatives I might be missing ? We don't want VIDEODEV depending on VB2. I think it also makes sense to have a separate Kconfig option for these helpers, are they will be used by a small number of drivers. Make CONFIG_V4L2_ISP an option that drivers select, not depend on. -- Regards, Laurent Pinchart
© 2016 - 2025 Red Hat, Inc.