On 08/03/2023 00.32, Maíra Canal wrote:
On 3/7/23 11:25, Asahi Lina wrote:
DRM drivers need to be able to declare which driver-specific ioctls they support. This abstraction adds the required types and a helper macro to generate the ioctl definition inside the DRM driver.
Note that this macro is not usable until further bits of the abstraction are in place (but it will not fail to compile on its own, if not called).
Signed-off-by: Asahi Lina lina@asahilina.net
drivers/gpu/drm/Kconfig | 7 ++ rust/bindings/bindings_helper.h | 2 + rust/kernel/drm/ioctl.rs | 147 ++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 5 ++ rust/kernel/lib.rs | 2 + 5 files changed, 163 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index dc0f94f02a82..dab8f0f9aa96 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -27,6 +27,13 @@ menuconfig DRM details. You should also select and configure AGP (/dev/agpgart) support if it is available for your platform.
[...]
+/// Declare the DRM ioctls for a driver. +/// +/// Each entry in the list should have the form: +/// +/// `(ioctl_number, argument_type, flags, user_callback),` +/// +/// `argument_type` is the type name within the `bindings` crate. +/// `user_callback` should have the following prototype: +/// +/// ``` +/// fn foo(device: &kernel::drm::device::Device<Self>, +/// data: &mut bindings::argument_type, +/// file: &kernel::drm::file::FileSelf::File, +/// ) +/// ``` +/// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within. +/// +/// # Examples +/// +/// ``` +/// kernel::declare_drm_ioctls! { +/// (FOO_GET_PARAM, drm_foo_get_param, ioctl::RENDER_ALLOW, my_get_param_handler), +/// } +/// ``` +/// +#[macro_export] +macro_rules! declare_drm_ioctls {
- ( $(($cmd:ident, $struct:ident, $flags:expr, $func:expr)),* $(,)? ) => {
const IOCTLS: &'static [$crate::drm::ioctl::DrmIoctlDescriptor] = {const _:() = {let i: u32 = $crate::bindings::DRM_COMMAND_BASE;// Assert that all the IOCTLs are in the right order and there are no gaps,// and that the sizeof of the specified type is correct.I believe that not necessarily the IOCTLs need to be in the right order and with no gaps. For example, armada_drm.h has a gap in between 0x00 and 0x02 and exynos_drm.h also have gaps. Moreover, some drivers, like vgem and virtgpu, start their IOCTLs with 0x01.
Yeah, we talked about this a bit... do you have any ideas about how to design this? I think it should be possible with a const function initializing an array entry by entry, we just need a two-pass macro (once to determine the max ioctl number, then again to actually output the implementation).
I'm not sure why drivers would have gaps in the ioctl numbers though... my idea was that new drivers shouldn't need that as far as I can tell (you can't remove APIs after the fact due to UAPI stability guarantees, so as long as you don't have gaps to begin with...). But I guess if we're reimplementing existing drivers in Rust we'll need this... though maybe it makes sense to just say it's not supported and require reimplementations that have holes to just explicitly add dummy ioctls that return EINVAL? We could even provide such a dummy generic ioctl handler on the abstraction side, so drivers just have to add it to the list, or make the macro take a special token that is used for placeholder ioctls that don't exist (which then creates the NULL function pointer that the drm core interprets as invalid)...
Basically I'm not sure if it makes sense to fully support noncontiguous ioctl numbers automagically, or just say drivers need to explicitly list gaps. I'd love to hear the opinion of other DRM folks about this!
~~ Lina