Le 14/02/2025 à 16:53, José Expósito a écrit :
On Thu, Feb 13, 2025 at 02:59:52PM +0100, Louis Chauvet wrote:
On 11/02/25 - 12:09, José Expósito wrote:
From: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
As the configuration will be used by userspace, add a validator to avoid
creating a broken DRM device.
For the moment, the function always returns true, but rules will be
added in future patches.
Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
Co-developed-by: José Expósito <jose.exposito89@xxxxxxxxx>
Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
The compilation is broken when building as module:
diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
index b9267aef4804..82335006c94a 100644
--- a/drivers/gpu/drm/vkms/vkms_config.c
+++ b/drivers/gpu/drm/vkms/vkms_config.c
@@ -55,6 +55,7 @@ bool vkms_config_is_valid(struct vkms_config *config)
{
return true;
}
+EXPORT_SYMBOL_IF_KUNIT(vkms_config_is_valid);
Fixed the issue in all patches, thanks!
[...]
diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
index fcaa909fb2e0..0376dceaf6be 100644
--- a/drivers/gpu/drm/vkms/vkms_config.h
+++ b/drivers/gpu/drm/vkms/vkms_config.h
@@ -67,6 +67,16 @@ vkms_config_get_device_name(struct vkms_config *config)
return config->dev_name;
}
+/**
+ * vkms_config_is_valid() - Validate a configuration
+ * @config: Configuration to validate
+ *
+ * Returns:
+ * Whether the configuration is valid or not.
+ * For example, a configuration without primary planes is not valid.
+ */
+bool vkms_config_is_valid(struct vkms_config *config);
+
I think here we can take a const pointer.
/**
* vkms_config_register_debugfs() - Register a debugfs file to show the device's
* configuration
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index a74a7fc3a056..95afc39ce985 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -204,7 +204,7 @@ struct vkms_config;
struct vkms_device {
struct drm_device drm;
struct platform_device *platform;
- const struct vkms_config *config;
+ struct vkms_config *config;
So we can keep a const pointer here (for me the device should never modify
vkms_config)
I tryed keeping the const pointer, but, since list_count_nodes() is used in
several valid_* functions and it takes a non-const pointer, it causes
warnings.
I did not see this issue, and obviously no casting is better!
But I don't understand why list_count_node needs a non-const pointer. I
just compile-tested an allyesconfig with a const parameter, it seems to
works without issue (no warning, at least on x86_64, I just started some
build on other architectures to check most of the drivers).
I will submit a patch next week to make the list_count_nodes parameter
const.
We can fix them with a cast:
n_planes = list_count_nodes((struct list_head *)&config->planes);
But I feel that keeping the "const" creates more issues than it solves.
Anyway, if you prefer this pointer to be const, I will change it in v3.
Please use a const pointer + cast, and we will remove it once the
list_count_nodes patch is merged.
Thanks,
Louis Chauvet
Jose
};
/*
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 068a7f87ecec..414cc933af41 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -16,6 +16,9 @@ int vkms_output_init(struct vkms_device *vkmsdev)
int writeback;
unsigned int n;
+ if (!vkms_config_is_valid(vkmsdev->config))
+ return -EINVAL;
+
/*
* Initialize used plane. One primary plane is required to perform the composition.
*
--
2.48.1
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
