From cb271c2edfd0ab7204d5ef3c9d5ae9a0710f5bf2 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:56 +0200 Subject: rust: device: implement impl_device_context_deref! The Deref hierarchy for device context generics is the same for every (bus specific) device. Implement those with a generic macro to avoid duplicated boiler plate code and ensure the correct Deref hierarchy for every device implementation. Co-developed-by: Benno Lossin Signed-off-by: Benno Lossin Reviewed-by: Christian Schrefl Link: https://lore.kernel.org/r/20250413173758.12068-2-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/pci.rs | 16 +++------------- rust/kernel/platform.rs | 17 +++-------------- 3 files changed, 50 insertions(+), 27 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 21b343a1dc4d..9520e054996d 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -235,6 +235,50 @@ mod private { impl DeviceContext for Core {} impl DeviceContext for Normal {} +/// # Safety +/// +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the +/// generic argument of `$device`. +#[doc(hidden)] +#[macro_export] +macro_rules! __impl_device_context_deref { + (unsafe { $device:ident, $src:ty => $dst:ty }) => { + impl ::core::ops::Deref for $device<$src> { + type Target = $device<$dst>; + + fn deref(&self) -> &Self::Target { + let ptr: *const Self = self; + + // CAST: `$device<$src>` and `$device<$dst>` transparently wrap the same type by the + // safety requirement of the macro. + let ptr = ptr.cast::(); + + // SAFETY: `ptr` was derived from `&self`. + unsafe { &*ptr } + } + } + }; +} + +/// Implement [`core::ops::Deref`] traits for allowed [`DeviceContext`] conversions of a (bus +/// specific) device. +/// +/// # Safety +/// +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the +/// generic argument of `$device`. +#[macro_export] +macro_rules! impl_device_context_deref { + (unsafe { $device:ident }) => { + // SAFETY: This macro has the exact same safety requirement as + // `__impl_device_context_deref!`. + ::kernel::__impl_device_context_deref!(unsafe { + $device, + $crate::device::Core => $crate::device::Normal + }); + }; +} + #[doc(hidden)] #[macro_export] macro_rules! dev_printk { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index c97d6d470b28..8474608e7a90 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -422,19 +422,9 @@ impl Device { } } -impl Deref for Device { - type Target = Device; - - fn deref(&self) -> &Self::Target { - let ptr: *const Self = self; - - // CAST: `Device` is a transparent wrapper of `Opaque`. - let ptr = ptr.cast::(); - - // SAFETY: `ptr` was derived from `&self`. - unsafe { &*ptr } - } -} +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); impl From<&Device> for ARef { fn from(dev: &Device) -> Self { diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 4917cb34e2fe..22590bdff7bb 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -16,7 +16,6 @@ use crate::{ use core::{ marker::PhantomData, - ops::Deref, ptr::{addr_of_mut, NonNull}, }; @@ -190,19 +189,9 @@ impl Device { } } -impl Deref for Device { - type Target = Device; - - fn deref(&self) -> &Self::Target { - let ptr: *const Self = self; - - // CAST: `Device` is a transparent wrapper of `Opaque`. - let ptr = ptr.cast::(); - - // SAFETY: `ptr` was derived from `&self`. - unsafe { &*ptr } - } -} +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); impl From<&Device> for ARef { fn from(dev: &Device) -> Self { -- cgit From fbb92b6a534081cabd75861ac9c7a8d29d8effda Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:57 +0200 Subject: rust: device: implement impl_device_context_into_aref! Implement a macro to implement all From conversions of a certain device to ARef. This avoids unnecessary boiler plate code for every device implementation. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-3-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 21 +++++++++++++++++++++ rust/kernel/pci.rs | 7 +------ rust/kernel/platform.rs | 9 ++------- 3 files changed, 24 insertions(+), 13 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 9520e054996d..7b96c99879a6 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -279,6 +279,27 @@ macro_rules! impl_device_context_deref { }; } +#[doc(hidden)] +#[macro_export] +macro_rules! __impl_device_context_into_aref { + ($src:ty, $device:tt) => { + impl ::core::convert::From<&$device<$src>> for $crate::types::ARef<$device> { + fn from(dev: &$device<$src>) -> Self { + (&**dev).into() + } + } + }; +} + +/// Implement [`core::convert::From`], such that all `&Device` can be converted to an +/// `ARef`. +#[macro_export] +macro_rules! impl_device_context_into_aref { + ($device:tt) => { + ::kernel::__impl_device_context_into_aref!($crate::device::Core, $device); + }; +} + #[doc(hidden)] #[macro_export] macro_rules! dev_printk { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 8474608e7a90..7c6ec05383e0 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -425,12 +425,7 @@ impl Device { // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); - -impl From<&Device> for ARef { - fn from(dev: &Device) -> Self { - (&**dev).into() - } -} +kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 22590bdff7bb..9476b717c425 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -10,7 +10,7 @@ use crate::{ of, prelude::*, str::CStr, - types::{ARef, ForeignOwnable, Opaque}, + types::{ForeignOwnable, Opaque}, ThisModule, }; @@ -192,12 +192,7 @@ impl Device { // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); - -impl From<&Device> for ARef { - fn from(dev: &Device) -> Self { - (&**dev).into() - } -} +kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { -- cgit From d32e4c24a7fe01195ba427c67fc15864279a3c0d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:58 +0200 Subject: rust: device: implement device context for Device Analogous to bus specific device, implement the DeviceContext generic for generic devices. This is used for APIs that work with generic devices (such as Devres) to evaluate the device's context. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 7b96c99879a6..53a13dd4d2d4 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -9,7 +9,7 @@ use crate::{ str::CStr, types::{ARef, Opaque}, }; -use core::{fmt, ptr}; +use core::{fmt, marker::PhantomData, ptr}; #[cfg(CONFIG_PRINTK)] use crate::c_str; @@ -42,7 +42,7 @@ use crate::c_str; /// `bindings::device::release` is valid to be called from any thread, hence `ARef` can be /// dropped from any thread. #[repr(transparent)] -pub struct Device(Opaque); +pub struct Device(Opaque, PhantomData); impl Device { /// Creates a new reference-counted abstraction instance of an existing `struct device` pointer. @@ -59,7 +59,9 @@ impl Device { // SAFETY: By the safety requirements ptr is valid unsafe { Self::as_ref(ptr) }.into() } +} +impl Device { /// Obtain the raw `struct device *`. pub(crate) fn as_raw(&self) -> *mut bindings::device { self.0.get() @@ -189,6 +191,11 @@ impl Device { } } +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); +kernel::impl_device_context_into_aref!(Device); + // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { fn inc_ref(&self) { -- cgit From da6c47c6cb45ffb392e016631c08098ad2a6d418 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:59 +0200 Subject: rust: platform: preserve device context in AsRef Since device::Device has a generic over its context, preserve this device context in AsRef. For instance, when calling platform::Device the new AsRef implementation returns device::Device. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 9476b717c425..b1c48cd95cd6 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -183,7 +183,7 @@ pub struct Device( PhantomData, ); -impl Device { +impl Device { fn as_raw(&self) -> *mut bindings::platform_device { self.0.get() } @@ -207,8 +207,8 @@ unsafe impl crate::types::AlwaysRefCounted for Device { } } -impl AsRef for Device { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid // `struct platform_device`. let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; -- cgit From 3edaefbf2b1beb9ae1cb2a842f455157b951e9f1 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:00 +0200 Subject: rust: pci: preserve device context in AsRef Since device::Device has a generic over its context, preserve this device context in AsRef. For instance, when calling pci::Device the new AsRef implementation returns device::Device. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-6-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 7c6ec05383e0..1234b0c4a403 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -360,11 +360,13 @@ impl Deref for Bar { } } -impl Device { +impl Device { fn as_raw(&self) -> *mut bindings::pci_dev { self.0.get() } +} +impl Device { /// Returns the PCI vendor ID. pub fn vendor_id(&self) -> u16 { // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. @@ -440,8 +442,8 @@ unsafe impl crate::types::AlwaysRefCounted for Device { } } -impl AsRef for Device { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid // `struct pci_dev`. let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; -- cgit From f933b7489ffca25c9a33b65441679ee3d2943024 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:01 +0200 Subject: rust: device: implement Bound device context The Bound device context indicates that a device is bound to a driver. It must be used for APIs that require the device to be bound, such as Devres or dma::CoherentAllocation. Implement Bound and add the corresponding Deref hierarchy, as well as the corresponding ARef conversion for this device context. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-7-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 53a13dd4d2d4..0353c5552769 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -232,13 +232,19 @@ pub struct Normal; /// any of the bus callbacks, such as `probe()`. pub struct Core; +/// The [`Bound`] context is the context of a bus specific device reference when it is guaranteed to +/// be bound for the duration of its lifetime. +pub struct Bound; + mod private { pub trait Sealed {} + impl Sealed for super::Bound {} impl Sealed for super::Core {} impl Sealed for super::Normal {} } +impl DeviceContext for Bound {} impl DeviceContext for Core {} impl DeviceContext for Normal {} @@ -281,7 +287,14 @@ macro_rules! impl_device_context_deref { // `__impl_device_context_deref!`. ::kernel::__impl_device_context_deref!(unsafe { $device, - $crate::device::Core => $crate::device::Normal + $crate::device::Core => $crate::device::Bound + }); + + // SAFETY: This macro has the exact same safety requirement as + // `__impl_device_context_deref!`. + ::kernel::__impl_device_context_deref!(unsafe { + $device, + $crate::device::Bound => $crate::device::Normal }); }; } @@ -304,6 +317,7 @@ macro_rules! __impl_device_context_into_aref { macro_rules! impl_device_context_into_aref { ($device:tt) => { ::kernel::__impl_device_context_into_aref!($crate::device::Core, $device); + ::kernel::__impl_device_context_into_aref!($crate::device::Bound, $device); }; } -- cgit From f2a399d7b67c4a6fc0f8e59d1a9eb484efc71b5c Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:02 +0200 Subject: rust: pci: move iomap_region() to impl Device Require the Bound device context to be able to call iomap_region() and iomap_region_sized(). Creating I/O mapping requires the device to be bound. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-8-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 1234b0c4a403..3664d35b8e79 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -390,7 +390,9 @@ impl Device { // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`. Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) }) } +} +impl Device { /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks /// can be performed on compile time for offsets (plus the requested type size) < SIZE. pub fn iomap_region_sized( -- cgit From f720efda2db5e609b32100c25d9cf383f082d945 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:03 +0200 Subject: rust: devres: require a bound device Require the Bound device context to be able to a new Devres container. This ensures that we can't register devres callbacks for unbound devices. Link: https://lore.kernel.org/r/20250413173758.12068-9-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index ddb1ce4a78d9..1e58f5d22044 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -8,7 +8,7 @@ use crate::{ alloc::Flags, bindings, - device::Device, + device::{Bound, Device}, error::{Error, Result}, ffi::c_void, prelude::*, @@ -45,7 +45,7 @@ struct DevresInner { /// # Example /// /// ```no_run -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}}; +/// # use kernel::{bindings, c_str, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}}; /// # use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. @@ -83,13 +83,10 @@ struct DevresInner { /// unsafe { Io::from_raw(&self.0) } /// } /// } -/// # fn no_run() -> Result<(), Error> { -/// # // SAFETY: Invalid usage; just for the example to get an `ARef` instance. -/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) }; -/// +/// # fn no_run(dev: &Device) -> Result<(), Error> { /// // SAFETY: Invalid usage for example purposes. /// let iomem = unsafe { IoMem::<{ core::mem::size_of::() }>::new(0xBAAAAAAD)? }; -/// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?; +/// let devres = Devres::new(dev, iomem, GFP_KERNEL)?; /// /// let res = devres.try_access().ok_or(ENXIO)?; /// res.write8(0x42, 0x0); @@ -99,7 +96,7 @@ struct DevresInner { pub struct Devres(Arc>); impl DevresInner { - fn new(dev: &Device, data: T, flags: Flags) -> Result>> { + fn new(dev: &Device, data: T, flags: Flags) -> Result>> { let inner = Arc::pin_init( pin_init!( DevresInner { dev: dev.into(), @@ -171,7 +168,7 @@ impl DevresInner { impl Devres { /// Creates a new [`Devres`] instance of the given `data`. The `data` encapsulated within the /// returned `Devres` instance' `data` will be revoked once the device is detached. - pub fn new(dev: &Device, data: T, flags: Flags) -> Result { + pub fn new(dev: &Device, data: T, flags: Flags) -> Result { let inner = DevresInner::new(dev, data, flags)?; Ok(Devres(inner)) @@ -179,7 +176,7 @@ impl Devres { /// Same as [`Devres::new`], but does not return a `Devres` instance. Instead the given `data` /// is owned by devres and will be revoked / dropped, once the device is detached. - pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result { + pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result { let _ = DevresInner::new(dev, data, flags)?; Ok(()) -- cgit From 7bd1710aac0571d4722f1429a9332c57b3a8feaf Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:04 +0200 Subject: rust: dma: require a bound device Require the Bound device context to be able to create new dma::CoherentAllocation instances. DMA memory allocations are only valid to be created for bound devices. Link: https://lore.kernel.org/r/20250413173758.12068-10-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/dma.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 8cdc76043ee7..605e01e35715 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -6,7 +6,7 @@ use crate::{ bindings, build_assert, - device::Device, + device::{Bound, Device}, error::code::*, error::Result, transmute::{AsBytes, FromBytes}, @@ -22,10 +22,10 @@ use crate::{ /// # Examples /// /// ``` -/// use kernel::device::Device; +/// # use kernel::device::{Bound, Device}; /// use kernel::dma::{attrs::*, CoherentAllocation}; /// -/// # fn test(dev: &Device) -> Result { +/// # fn test(dev: &Device) -> Result { /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN; /// let c: CoherentAllocation = /// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?; @@ -143,16 +143,16 @@ impl CoherentAllocation { /// # Examples /// /// ``` - /// use kernel::device::Device; + /// # use kernel::device::{Bound, Device}; /// use kernel::dma::{attrs::*, CoherentAllocation}; /// - /// # fn test(dev: &Device) -> Result { + /// # fn test(dev: &Device) -> Result { /// let c: CoherentAllocation = /// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// # Ok::<(), Error>(()) } /// ``` pub fn alloc_attrs( - dev: &Device, + dev: &Device, count: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, @@ -194,7 +194,7 @@ impl CoherentAllocation { /// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the /// `dma_attrs` is 0 by default. pub fn alloc_coherent( - dev: &Device, + dev: &Device, count: usize, gfp_flags: kernel::alloc::Flags, ) -> Result> { -- cgit From a095d0d1e4849ee25c28d43d713a8f433d7f1f27 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 21 Mar 2025 22:47:54 +0100 Subject: rust: pci: impl TryFrom<&Device> for &pci::Device Implement TryFrom<&device::Device> for &Device. This allows us to get a &pci::Device from a generic &Device in a safe way; the conversion fails if the device' bus type does not match with the PCI bus type. Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250321214826.140946-3-dakr@kernel.org [ Support device context types, use dev_is_pci() helper. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 3664d35b8e79..38fc8d5ffbf9 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -6,7 +6,7 @@ use crate::{ alloc::flags::*, - bindings, device, + bindings, container_of, device, device_id::RawDeviceId, devres::Devres, driver, @@ -455,6 +455,26 @@ impl AsRef> for Device { } } +impl TryFrom<&device::Device> for &Device { + type Error = kernel::error::Error; + + fn try_from(dev: &device::Device) -> Result { + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a + // `struct device`. + if !unsafe { bindings::dev_is_pci(dev.as_raw()) } { + return Err(EINVAL); + } + + // SAFETY: We've just verified that the bus type of `dev` equals `bindings::pci_bus_type`, + // hence `dev` must be embedded in a valid `struct pci_dev` as guaranteed by the + // corresponding C code. + let pdev = unsafe { container_of!(dev.as_raw(), bindings::pci_dev, dev) }; + + // SAFETY: `pdev` is a valid pointer to a `struct pci_dev`. + Ok(unsafe { &*pdev.cast() }) + } +} + // SAFETY: A `Device` is always reference-counted and can be released from any thread. unsafe impl Send for Device {} -- cgit From a38dfd60fe53a46a93517f02a061bb34c47946dc Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 21 Mar 2025 22:47:55 +0100 Subject: rust: platform: impl TryFrom<&Device> for &platform::Device Implement TryFrom<&device::Device> for &Device. This allows us to get a &platform::Device from a generic &Device in a safe way; the conversion fails if the device' bus type does not match with the platform bus type. Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250321214826.140946-4-dakr@kernel.org [ Support device context types, use dev_is_platform() helper. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index b1c48cd95cd6..08849d92c074 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) use crate::{ - bindings, device, driver, + bindings, container_of, device, driver, error::{to_result, Result}, of, prelude::*, @@ -218,6 +218,26 @@ impl AsRef> for Device { } } +impl TryFrom<&device::Device> for &Device { + type Error = kernel::error::Error; + + fn try_from(dev: &device::Device) -> Result { + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a + // `struct device`. + if !unsafe { bindings::dev_is_platform(dev.as_raw()) } { + return Err(EINVAL); + } + + // SAFETY: We've just verified that the bus type of `dev` equals + // `bindings::platform_bus_type`, hence `dev` must be embedded in a valid + // `struct platform_device` as guaranteed by the corresponding C code. + let pdev = unsafe { container_of!(dev.as_raw(), bindings::platform_device, dev) }; + + // SAFETY: `pdev` is a valid pointer to a `struct platform_device`. + Ok(unsafe { &*pdev.cast() }) + } +} + // SAFETY: A `Device` is always reference-counted and can be released from any thread. unsafe impl Send for Device {} -- cgit From 9647b6c5095aa54701df009a7335d07013cd13c4 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:04 +0200 Subject: rust: types: add `Opaque::zeroed` Analogous to `Opaque::uninit` add `Opaque::zeroed`, which sets the corresponding memory to zero. In contrast to `Opaque::uninit`, the corresponding value, depending on its type, may be initialized. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250414131934.28418-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/types.rs | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 9d0471afc964..eee387727d1a 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -329,6 +329,14 @@ impl Opaque { } } + /// Creates a new zeroed opaque value. + pub const fn zeroed() -> Self { + Self { + value: UnsafeCell::new(MaybeUninit::zeroed()), + _pin: PhantomPinned, + } + } + /// Create an opaque pin-initializer from the given pin-initializer. pub fn pin_init(slot: impl PinInit) -> impl PinInit { Self::ffi_init(|ptr: *mut T| { -- cgit From a4c9f71e3440dc6e944fa05bb7fcb3949fe5bcd4 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:05 +0200 Subject: rust: device: implement Device::parent() Device::parent() returns a reference to the device' parent device, if any. Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 0353c5552769..0d237cebd65e 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -67,6 +67,25 @@ impl Device { self.0.get() } + /// Returns a reference to the parent device, if any. + #[expect(unused)] + pub(crate) fn parent(&self) -> Option<&Self> { + // SAFETY: + // - By the type invariant `self.as_raw()` is always valid. + // - The parent device is only ever set at device creation. + let parent = unsafe { (*self.as_raw()).parent }; + + if parent.is_null() { + None + } else { + // SAFETY: + // - Since `parent` is not NULL, it must be a valid pointer to a `struct device`. + // - `parent` is valid for the lifetime of `self`, since a `struct device` holds a + // reference count of its parent. + Some(unsafe { Self::as_ref(parent) }) + } + } + /// Convert a raw C `struct device` pointer to a `&'a Device`. /// /// # Safety -- cgit From ce735e73dd59b169b877cedd0753297c81c2a091 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:06 +0200 Subject: rust: auxiliary: add auxiliary device / driver abstractions Implement the basic auxiliary abstractions required to implement a driver matching an auxiliary device. The design and implementation is analogous to PCI and platform and is based on the generic device / driver abstractions. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-4-dakr@kernel.org [ Fix typos, `let _ =` => `drop()`, use `kernel::ffi`. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 274 +++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/device.rs | 1 - rust/kernel/lib.rs | 2 + 3 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 rust/kernel/auxiliary.rs (limited to 'rust/kernel') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs new file mode 100644 index 000000000000..e15596263c22 --- /dev/null +++ b/rust/kernel/auxiliary.rs @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Abstractions for the auxiliary bus. +//! +//! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) + +use crate::{ + bindings, device, + device_id::RawDeviceId, + driver, + error::{to_result, Result}, + prelude::*, + str::CStr, + types::{ForeignOwnable, Opaque}, + ThisModule, +}; +use core::{ + marker::PhantomData, + ptr::{addr_of_mut, NonNull}, +}; + +/// An adapter for the registration of auxiliary drivers. +pub struct Adapter(T); + +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if +// a preceding call to `register` has been successful. +unsafe impl driver::RegistrationOps for Adapter { + type RegType = bindings::auxiliary_driver; + + unsafe fn register( + adrv: &Opaque, + name: &'static CStr, + module: &'static ThisModule, + ) -> Result { + // SAFETY: It's safe to set the fields of `struct auxiliary_driver` on initialization. + unsafe { + (*adrv.get()).name = name.as_char_ptr(); + (*adrv.get()).probe = Some(Self::probe_callback); + (*adrv.get()).remove = Some(Self::remove_callback); + (*adrv.get()).id_table = T::ID_TABLE.as_ptr(); + } + + // SAFETY: `adrv` is guaranteed to be a valid `RegType`. + to_result(unsafe { + bindings::__auxiliary_driver_register(adrv.get(), module.0, name.as_char_ptr()) + }) + } + + unsafe fn unregister(adrv: &Opaque) { + // SAFETY: `adrv` is guaranteed to be a valid `RegType`. + unsafe { bindings::auxiliary_driver_unregister(adrv.get()) } + } +} + +impl Adapter { + extern "C" fn probe_callback( + adev: *mut bindings::auxiliary_device, + id: *const bindings::auxiliary_device_id, + ) -> kernel::ffi::c_int { + // SAFETY: The auxiliary bus only ever calls the probe callback with a valid pointer to a + // `struct auxiliary_device`. + // + // INVARIANT: `adev` is valid for the duration of `probe_callback()`. + let adev = unsafe { &*adev.cast::>() }; + + // SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_device_id` + // and does not add additional invariants, so it's safe to transmute. + let id = unsafe { &*id.cast::() }; + let info = T::ID_TABLE.info(id.index()); + + match T::probe(adev, info) { + Ok(data) => { + // Let the `struct auxiliary_device` own a reference of the driver's private data. + // SAFETY: By the type invariant `adev.as_raw` returns a valid pointer to a + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_set_drvdata(adev.as_raw(), data.into_foreign()) }; + } + Err(err) => return Error::to_errno(err), + } + + 0 + } + + extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) { + // SAFETY: The auxiliary bus only ever calls the remove callback with a valid pointer to a + // `struct auxiliary_device`. + let ptr = unsafe { bindings::auxiliary_get_drvdata(adev) }; + + // SAFETY: `remove_callback` is only ever called after a successful call to + // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized + // `KBox` pointer created through `KBox::into_foreign`. + drop(unsafe { KBox::::from_foreign(ptr) }); + } +} + +/// Declares a kernel module that exposes a single auxiliary driver. +#[macro_export] +macro_rules! module_auxiliary_driver { + ($($f:tt)*) => { + $crate::module_driver!(, $crate::auxiliary::Adapter, { $($f)* }); + }; +} + +/// Abstraction for `bindings::auxiliary_device_id`. +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::auxiliary_device_id); + +impl DeviceId { + /// Create a new [`DeviceId`] from name. + pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self { + let name = name.as_bytes_with_nul(); + let modname = modname.as_bytes_with_nul(); + + // TODO: Replace with `bindings::auxiliary_device_id::default()` once stabilized for + // `const`. + // + // SAFETY: FFI type is valid to be zero-initialized. + let mut id: bindings::auxiliary_device_id = unsafe { core::mem::zeroed() }; + + let mut i = 0; + while i < modname.len() { + id.name[i] = modname[i]; + i += 1; + } + + // Reuse the space of the NULL terminator. + id.name[i - 1] = b'.'; + + let mut j = 0; + while j < name.len() { + id.name[i] = name[j]; + i += 1; + j += 1; + } + + Self(id) + } +} + +// SAFETY: +// * `DeviceId` is a `#[repr(transparent)`] wrapper of `auxiliary_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. +// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::auxiliary_device_id; + + const DRIVER_DATA_OFFSET: usize = + core::mem::offset_of!(bindings::auxiliary_device_id, driver_data); + + fn index(&self) -> usize { + self.0.driver_data + } +} + +/// IdTable type for auxiliary drivers. +pub type IdTable = &'static dyn kernel::device_id::IdTable; + +/// Create a auxiliary `IdTable` with its alias for modpost. +#[macro_export] +macro_rules! auxiliary_device_table { + ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => { + const $table_name: $crate::device_id::IdArray< + $crate::auxiliary::DeviceId, + $id_info_type, + { $table_data.len() }, + > = $crate::device_id::IdArray::new($table_data); + + $crate::module_device_table!("auxiliary", $module_table_name, $table_name); + }; +} + +/// The auxiliary driver trait. +/// +/// Drivers must implement this trait in order to get an auxiliary driver registered. +pub trait Driver { + /// The type holding information about each device id supported by the driver. + /// + /// TODO: Use associated_type_defaults once stabilized: + /// + /// type IdInfo: 'static = (); + type IdInfo: 'static; + + /// The table of device ids supported by the driver. + const ID_TABLE: IdTable; + + /// Auxiliary driver probe. + /// + /// Called when an auxiliary device is matches a corresponding driver. + fn probe(dev: &Device, id_info: &Self::IdInfo) -> Result>>; +} + +/// The auxiliary device representation. +/// +/// This structure represents the Rust abstraction for a C `struct auxiliary_device`. The +/// implementation abstracts the usage of an already existing C `struct auxiliary_device` within +/// Rust code that we get passed from the C side. +/// +/// # Invariants +/// +/// A [`Device`] instance represents a valid `struct auxiliary_device` created by the C portion of +/// the kernel. +#[repr(transparent)] +pub struct Device( + Opaque, + PhantomData, +); + +impl Device { + fn as_raw(&self) -> *mut bindings::auxiliary_device { + self.0.get() + } + + /// Returns the auxiliary device' id. + pub fn id(&self) -> u32 { + // SAFETY: By the type invariant `self.as_raw()` is a valid pointer to a + // `struct auxiliary_device`. + unsafe { (*self.as_raw()).id } + } + + /// Returns a reference to the parent [`device::Device`], if any. + pub fn parent(&self) -> Option<&device::Device> { + let ptr: *const Self = self; + // CAST: `Device` types are transparent to each other. + let ptr: *const Device = ptr.cast(); + // SAFETY: `ptr` was derived from `&self`. + let this = unsafe { &*ptr }; + + this.as_ref().parent() + } +} + +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); +kernel::impl_device_context_into_aref!(Device); + +// SAFETY: Instances of `Device` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Device { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::get_device(self.as_ref().as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // CAST: `Self` a transparent wrapper of `bindings::auxiliary_device`. + let adev: *mut bindings::auxiliary_device = obj.cast().as_ptr(); + + // SAFETY: By the type invariant of `Self`, `adev` is a pointer to a valid + // `struct auxiliary_device`. + let dev = unsafe { addr_of_mut!((*adev).dev) }; + + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::put_device(dev) } + } +} + +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid + // `struct auxiliary_device`. + let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; + + // SAFETY: `dev` points to a valid `struct device`. + unsafe { device::Device::as_ref(dev) } + } +} + +// SAFETY: A `Device` is always reference-counted and can be released from any thread. +unsafe impl Send for Device {} + +// SAFETY: `Device` can be shared among threads because all methods of `Device` +// (i.e. `Device) are thread safe. +unsafe impl Sync for Device {} diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 0d237cebd65e..40c1f549b0ba 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -68,7 +68,6 @@ impl Device { } /// Returns a reference to the parent device, if any. - #[expect(unused)] pub(crate) fn parent(&self) -> Option<&Self> { // SAFETY: // - By the type invariant `self.as_raw()` is always valid. diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index de07aadd1ff5..55a8dfeece0b 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -38,6 +38,8 @@ extern crate self as kernel; pub use ffi; pub mod alloc; +#[cfg(CONFIG_AUXILIARY_BUS)] +pub mod auxiliary; #[cfg(CONFIG_BLOCK)] pub mod block; #[doc(hidden)] -- cgit From 0d1803d25f8c7586beb3843c8efbb83f24ce2539 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:07 +0200 Subject: rust: auxiliary: add auxiliary registration Implement the `auxiliary::Registration` type, which provides an API to create and register new auxiliary devices in the system. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e15596263c22..5c072960dee0 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) use crate::{ - bindings, device, + bindings, container_of, device, device_id::RawDeviceId, driver, error::{to_result, Result}, @@ -230,6 +230,18 @@ impl Device { } } +impl Device { + extern "C" fn release(dev: *mut bindings::device) { + // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` + // embedded in `struct auxiliary_device`. + let adev = unsafe { container_of!(dev, bindings::auxiliary_device, dev) }.cast_mut(); + + // SAFETY: `adev` points to the memory that has been allocated in `Registration::new`, via + // `KBox::new(Opaque::::zeroed(), GFP_KERNEL)`. + let _ = unsafe { KBox::>::from_raw(adev.cast()) }; + } +} + // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); @@ -272,3 +284,77 @@ unsafe impl Send for Device {} // SAFETY: `Device` can be shared among threads because all methods of `Device` // (i.e. `Device) are thread safe. unsafe impl Sync for Device {} + +/// The registration of an auxiliary device. +/// +/// This type represents the registration of a [`struct auxiliary_device`]. When an instance of this +/// type is dropped, its respective auxiliary device will be unregistered from the system. +/// +/// # Invariants +/// +/// `self.0` always holds a valid pointer to an initialized and registered +/// [`struct auxiliary_device`]. +pub struct Registration(NonNull); + +impl Registration { + /// Create and register a new auxiliary device. + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result { + let boxed = KBox::new(Opaque::::zeroed(), GFP_KERNEL)?; + let adev = boxed.get(); + + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. + unsafe { + (*adev).dev.parent = parent.as_raw(); + (*adev).dev.release = Some(Device::release); + (*adev).name = name.as_char_ptr(); + (*adev).id = id; + } + + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has not been initialized yet. + unsafe { bindings::auxiliary_device_init(adev) }; + + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. + let _ = KBox::into_raw(boxed); + + // SAFETY: + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has + // been initialialized, + // - `modname.as_char_ptr()` is a NULL terminated string. + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; + if ret != 0 { + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has been initialialized. + unsafe { bindings::auxiliary_device_uninit(adev) }; + + return Err(Error::from_errno(ret)); + } + + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated successfully. + // + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is called, + // which happens in `Self::drop()`. + Ok(Self(unsafe { NonNull::new_unchecked(adev) })) + } +} + +impl Drop for Registration { + fn drop(&mut self) { + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) }; + + // This drops the reference we acquired through `auxiliary_device_init()`. + // + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) }; + } +} + +// SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread. +unsafe impl Send for Registration {} + +// SAFETY: `Registration` does not expose any methods or fields that need synchronization. +unsafe impl Sync for Registration {} -- cgit From 80e62fcea4f3ce8c7cc3205d7543e532b255d322 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Fri, 11 Apr 2025 21:09:38 +0900 Subject: rust/revocable: add try_access_with() convenience method Revocable::try_access() returns a guard through which the wrapped object can be accessed. Code that can sleep is not allowed while the guard is held; thus, it is common for the caller to explicitly drop it before running sleepable code, e.g: let b = bar.try_access()?; let reg = b.readl(...); // Don't forget this or things could go wrong! drop(b); something_that_might_sleep(); let b = bar.try_access()?; let reg2 = b.readl(...); This is arguably error-prone. try_access_with() provides an arguably safer alternative, by taking a closure that is run while the guard is held, and by dropping the guard automatically after the closure completes. This way, code can be organized more clearly around the critical sections and the risk of forgetting to release the guard when needed is considerably reduced: let reg = bar.try_access_with(|b| b.readl(...))?; something_that_might_sleep(); let reg2 = bar.try_access_with(|b| b.readl(...))?; The closure can return nothing, or any value including a Result which is then wrapped inside the Option returned by try_access_with. Error management is driver-specific, so users are encouraged to create their own macros that map and flatten the returned values to something appropriate for the code they are working on. Suggested-by: Danilo Krummrich Reviewed-by: Benno Lossin Signed-off-by: Alexandre Courbot Reviewed-by: Joel Fernandes Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250411-try_with-v4-1-f470ac79e2e2@nvidia.com [ Link `None`, `Some`, `Option` in doc-comment. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index 1e5a9d25c21b..971d0dc38d83 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -123,6 +123,22 @@ impl Revocable { } } + /// Tries to access the wrapped object and run a closure on it while the guard is held. + /// + /// This is a convenience method to run short non-sleepable code blocks while ensuring the + /// guard is dropped afterwards. [`Self::try_access`] carries the risk that the caller will + /// forget to explicitly drop that returned guard before calling sleepable code; this method + /// adds an extra safety to make sure it doesn't happen. + /// + /// Returns [`None`] if the object has been revoked and is therefore no longer accessible, or + /// the result of the closure wrapped in [`Some`]. If the closure returns a [`Result`] then the + /// return type becomes `Option>`, which can be inconvenient. Users are encouraged to + /// define their own macro that turns the [`Option`] into a proper error code and flattens the + /// inner result into it if it makes sense within their subsystem. + pub fn try_access_with R>(&self, f: F) -> Option { + self.try_access().map(|t| f(&*t)) + } + /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. -- cgit From 9a69570682b1f179a8bd9439a24495e7a6246aa9 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:21 +0200 Subject: rust: drm: ioctl: Add DRM ioctl abstraction DRM drivers need to be able to declare which driver-specific ioctls they support. Add an abstraction implementing 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 Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-3-dakr@kernel.org [ MISC fixes * wrap raw_data in Opaque to avoid UB when creating a reference * fix IOCTL sample declaration * fix safety comment of IOCTL argument * original source archive: https://archive.is/LqHDQ - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/ioctl.rs | 162 +++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 5 ++ rust/kernel/lib.rs | 2 + 3 files changed, 169 insertions(+) create mode 100644 rust/kernel/drm/ioctl.rs create mode 100644 rust/kernel/drm/mod.rs (limited to 'rust/kernel') diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs new file mode 100644 index 000000000000..445639404fb7 --- /dev/null +++ b/rust/kernel/drm/ioctl.rs @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM IOCTL definitions. +//! +//! C header: [`include/linux/drm/drm_ioctl.h`](srctree/include/linux/drm/drm_ioctl.h) + +use crate::ioctl; + +const BASE: u32 = uapi::DRM_IOCTL_BASE as u32; + +/// Construct a DRM ioctl number with no argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IO(nr: u32) -> u32 { + ioctl::_IO(BASE, nr) +} + +/// Construct a DRM ioctl number with a read-only argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOR(nr: u32) -> u32 { + ioctl::_IOR::(BASE, nr) +} + +/// Construct a DRM ioctl number with a write-only argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOW(nr: u32) -> u32 { + ioctl::_IOW::(BASE, nr) +} + +/// Construct a DRM ioctl number with a read-write argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOWR(nr: u32) -> u32 { + ioctl::_IOWR::(BASE, nr) +} + +/// Descriptor type for DRM ioctls. Use the `declare_drm_ioctls!{}` macro to construct them. +pub type DrmIoctlDescriptor = bindings::drm_ioctl_desc; + +/// This is for ioctl which are used for rendering, and require that the file descriptor is either +/// for a render node, or if it’s a legacy/primary node, then it must be authenticated. +pub const AUTH: u32 = bindings::drm_ioctl_flags_DRM_AUTH; + +/// This must be set for any ioctl which can change the modeset or display state. Userspace must +/// call the ioctl through a primary node, while it is the active master. +/// +/// Note that read-only modeset ioctl can also be called by unauthenticated clients, or when a +/// master is not the currently active one. +pub const MASTER: u32 = bindings::drm_ioctl_flags_DRM_MASTER; + +/// Anything that could potentially wreak a master file descriptor needs to have this flag set. +/// +/// Current that’s only for the SETMASTER and DROPMASTER ioctl, which e.g. logind can call to +/// force a non-behaving master (display compositor) into compliance. +/// +/// This is equivalent to callers with the SYSADMIN capability. +pub const ROOT_ONLY: u32 = bindings::drm_ioctl_flags_DRM_ROOT_ONLY; + +/// This is used for all ioctl needed for rendering only, for drivers which support render nodes. +/// This should be all new render drivers, and hence it should be always set for any ioctl with +/// `AUTH` set. Note though that read-only query ioctl might have this set, but have not set +/// DRM_AUTH because they do not require authentication. +pub const RENDER_ALLOW: u32 = bindings::drm_ioctl_flags_DRM_RENDER_ALLOW; + +/// Internal structures used by the `declare_drm_ioctls!{}` macro. Do not use directly. +#[doc(hidden)] +pub mod internal { + pub use bindings::drm_device; + pub use bindings::drm_file; + pub use bindings::drm_ioctl_desc; +} + +/// 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: +/// +/// ```ignore +/// fn foo(device: &kernel::drm::Device, +/// data: &Opaque, +/// file: &kernel::drm::File, +/// ) -> Result +/// ``` +/// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within. +/// +/// # Examples +/// +/// ```ignore +/// 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] = { + use $crate::uapi::*; + const _:() = { + let i: u32 = $crate::uapi::DRM_COMMAND_BASE; + // Assert that all the IOCTLs are in the right order and there are no gaps, + // and that the size of the specified type is correct. + $( + let cmd: u32 = $crate::macros::concat_idents!(DRM_IOCTL_, $cmd); + ::core::assert!(i == $crate::ioctl::_IOC_NR(cmd)); + ::core::assert!(core::mem::size_of::<$crate::uapi::$struct>() == + $crate::ioctl::_IOC_SIZE(cmd)); + let i: u32 = i + 1; + )* + }; + + let ioctls = &[$( + $crate::drm::ioctl::internal::drm_ioctl_desc { + cmd: $crate::macros::concat_idents!(DRM_IOCTL_, $cmd) as u32, + func: { + #[allow(non_snake_case)] + unsafe extern "C" fn $cmd( + raw_dev: *mut $crate::drm::ioctl::internal::drm_device, + raw_data: *mut ::core::ffi::c_void, + raw_file: *mut $crate::drm::ioctl::internal::drm_file, + ) -> core::ffi::c_int { + // SAFETY: + // - The DRM core ensures the device lives while callbacks are being + // called. + // - The DRM device must have been registered when we're called through + // an IOCTL. + // + // FIXME: Currently there is nothing enforcing that the types of the + // dev/file match the current driver these ioctls are being declared + // for, and it's not clear how to enforce this within the type system. + let dev = $crate::drm::device::Device::as_ref(raw_dev); + // SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we + // asserted above matches the size of this type, and all bit patterns of + // UAPI structs must be valid. + let data = unsafe { + &*(raw_data as *const $crate::types::Opaque<$crate::uapi::$struct>) + }; + // SAFETY: This is just the DRM file structure + let file = unsafe { $crate::drm::File::as_ref(raw_file) }; + + match $func(dev, data, file) { + Err(e) => e.to_errno(), + Ok(i) => i.try_into() + .unwrap_or($crate::error::code::ERANGE.to_errno()), + } + } + Some($cmd) + }, + flags: $flags, + name: $crate::c_str!(::core::stringify!($cmd)).as_char_ptr(), + } + ),*]; + ioctls + }; + }; +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs new file mode 100644 index 000000000000..9ec6d7cbcaf3 --- /dev/null +++ b/rust/kernel/drm/mod.rs @@ -0,0 +1,5 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM subsystem abstractions. + +pub mod ioctl; diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 55a8dfeece0b..ab0286857061 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -50,6 +50,8 @@ pub mod device_id; pub mod devres; pub mod dma; pub mod driver; +#[cfg(CONFIG_DRM = "y")] +pub mod drm; pub mod error; pub mod faux; #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)] -- cgit From 07c9016085f95fe9ad90079753f156859c54f476 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:22 +0200 Subject: rust: drm: add driver abstractions Implement the DRM driver abstractions. The `Driver` trait provides the interface to the actual driver to fill in the driver specific data, such as the `DriverInfo`, driver features and IOCTLs. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-4-dakr@kernel.org [ MISC changes * remove unnecessary DRM features; make remaining ones crate private * add #[expect(unused)] to avoid warnings * add sealed trait * remove shmem::Object references * original source archive: https://archive.is/Pl9ys - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/driver.rs | 113 ++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 8 ++++ 2 files changed, 121 insertions(+) create mode 100644 rust/kernel/drm/driver.rs (limited to 'rust/kernel') diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs new file mode 100644 index 000000000000..c85cf5e4cd7c --- /dev/null +++ b/rust/kernel/drm/driver.rs @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM driver core. +//! +//! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h) + +use crate::{bindings, drm, str::CStr}; +use macros::vtable; + +/// Driver use the GEM memory manager. This should be set for all modern drivers. +#[expect(unused)] +pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM; + +/// Information data for a DRM Driver. +pub struct DriverInfo { + /// Driver major version. + pub major: i32, + /// Driver minor version. + pub minor: i32, + /// Driver patchlevel version. + pub patchlevel: i32, + /// Driver name. + pub name: &'static CStr, + /// Driver description. + pub desc: &'static CStr, +} + +/// Internal memory management operation set, normally created by memory managers (e.g. GEM). +pub struct AllocOps { + #[expect(unused)] + pub(crate) gem_create_object: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + size: usize, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) prime_handle_to_fd: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + file_priv: *mut bindings::drm_file, + handle: u32, + flags: u32, + prime_fd: *mut core::ffi::c_int, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) prime_fd_to_handle: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + file_priv: *mut bindings::drm_file, + prime_fd: core::ffi::c_int, + handle: *mut u32, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) gem_prime_import: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + dma_buf: *mut bindings::dma_buf, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) gem_prime_import_sg_table: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + attach: *mut bindings::dma_buf_attachment, + sgt: *mut bindings::sg_table, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) dumb_create: Option< + unsafe extern "C" fn( + file_priv: *mut bindings::drm_file, + dev: *mut bindings::drm_device, + args: *mut bindings::drm_mode_create_dumb, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) dumb_map_offset: Option< + unsafe extern "C" fn( + file_priv: *mut bindings::drm_file, + dev: *mut bindings::drm_device, + handle: u32, + offset: *mut u64, + ) -> core::ffi::c_int, + >, +} + +/// Trait for memory manager implementations. Implemented internally. +pub trait AllocImpl: super::private::Sealed { + /// The C callback operations for this memory manager. + const ALLOC_OPS: AllocOps; +} + +/// The DRM `Driver` trait. +/// +/// This trait must be implemented by drivers in order to create a `struct drm_device` and `struct +/// drm_driver` to be registered in the DRM subsystem. +#[vtable] +pub trait Driver { + /// Context data associated with the DRM driver + type Data: Sync + Send; + + /// The type used to manage memory for this driver. + type Object: AllocImpl; + + /// Driver metadata + const INFO: DriverInfo; + + /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`. + const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor]; +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 9ec6d7cbcaf3..2e3f9a8a9353 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -2,4 +2,12 @@ //! DRM subsystem abstractions. +pub mod driver; pub mod ioctl; + +pub use self::driver::Driver; +pub use self::driver::DriverInfo; + +pub(crate) mod private { + pub trait Sealed {} +} -- cgit From 1e4b8896c0f3cb305a32870e1d8624f1155072d5 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:23 +0200 Subject: rust: drm: add device abstraction Implement the abstraction for a `struct drm_device`. A `drm::Device` creates a static const `struct drm_driver` filled with the data from the `drm::Driver` trait implementation of the actual driver creating the `drm::Device`. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-5-dakr@kernel.org [ Rewrite of drm::Device * full rewrite of the drm::Device abstraction using the subclassing pattern * original source archive: http://archive.today/5NxBo - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 198 ++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/driver.rs | 8 -- rust/kernel/drm/mod.rs | 2 + 3 files changed, 200 insertions(+), 8 deletions(-) create mode 100644 rust/kernel/drm/device.rs (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs new file mode 100644 index 000000000000..d6c6cecbdd51 --- /dev/null +++ b/rust/kernel/drm/device.rs @@ -0,0 +1,198 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM device. +//! +//! C header: [`include/linux/drm/drm_device.h`](srctree/include/linux/drm/drm_device.h) + +use crate::{ + bindings, device, drm, + drm::driver::AllocImpl, + error::from_err_ptr, + error::Result, + prelude::*, + types::{ARef, AlwaysRefCounted, Opaque}, +}; +use core::{mem, ops::Deref, ptr, ptr::NonNull}; + +#[cfg(CONFIG_DRM_LEGACY)] +macro_rules! drm_legacy_fields { + ( $($field:ident: $val:expr),* $(,)? ) => { + bindings::drm_driver { + $( $field: $val ),*, + firstopen: None, + preclose: None, + dma_ioctl: None, + dma_quiescent: None, + context_dtor: None, + irq_handler: None, + irq_preinstall: None, + irq_postinstall: None, + irq_uninstall: None, + get_vblank_counter: None, + enable_vblank: None, + disable_vblank: None, + dev_priv_size: 0, + } + } +} + +#[cfg(not(CONFIG_DRM_LEGACY))] +macro_rules! drm_legacy_fields { + ( $($field:ident: $val:expr),* $(,)? ) => { + bindings::drm_driver { + $( $field: $val ),* + } + } +} + +/// A typed DRM device with a specific `drm::Driver` implementation. +/// +/// The device is always reference-counted. +/// +/// # Invariants +/// +/// `self.dev` is a valid instance of a `struct device`. +#[repr(C)] +#[pin_data] +pub struct Device { + dev: Opaque, + #[pin] + data: T::Data, +} + +impl Device { + const VTABLE: bindings::drm_driver = drm_legacy_fields! { + load: None, + open: None, // TODO: File abstraction + postclose: None, // TODO: File abstraction + unload: None, + release: None, + master_set: None, + master_drop: None, + debugfs_init: None, + gem_create_object: T::Object::ALLOC_OPS.gem_create_object, + prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd, + prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle, + gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import, + gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table, + dumb_create: T::Object::ALLOC_OPS.dumb_create, + dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset, + show_fdinfo: None, + fbdev_probe: None, + + major: T::INFO.major, + minor: T::INFO.minor, + patchlevel: T::INFO.patchlevel, + name: T::INFO.name.as_char_ptr() as *mut _, + desc: T::INFO.desc.as_char_ptr() as *mut _, + + driver_features: drm::driver::FEAT_GEM, + ioctls: T::IOCTLS.as_ptr(), + num_ioctls: T::IOCTLS.len() as i32, + fops: core::ptr::null_mut() as _, + }; + + /// Create a new `drm::Device` for a `drm::Driver`. + pub fn new(dev: &device::Device, data: impl PinInit) -> Result> { + // SAFETY: + // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation, + // - `dev` is valid by its type invarants, + let raw_drm: *mut Self = unsafe { + bindings::__drm_dev_alloc( + dev.as_raw(), + &Self::VTABLE, + mem::size_of::(), + mem::offset_of!(Self, dev), + ) + } + .cast(); + let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?; + + // SAFETY: `raw_drm` is a valid pointer to `Self`. + let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) }; + + // SAFETY: + // - `raw_data` is a valid pointer to uninitialized memory. + // - `raw_data` will not move until it is dropped. + unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| { + // SAFETY: `__drm_dev_alloc()` was successful, hence `raw_drm` must be valid and the + // refcount must be non-zero. + unsafe { bindings::drm_dev_put(ptr::addr_of_mut!((*raw_drm.as_ptr()).dev).cast()) }; + })?; + + // SAFETY: The reference count is one, and now we take ownership of that reference as a + // `drm::Device`. + Ok(unsafe { ARef::from_raw(raw_drm) }) + } + + pub(crate) fn as_raw(&self) -> *mut bindings::drm_device { + self.dev.get() + } + + /// # Safety + /// + /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`. + unsafe fn from_drm_device(ptr: *const bindings::drm_device) -> *mut Self { + // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a + // `struct drm_device` embedded in `Self`. + unsafe { crate::container_of!(ptr, Self, dev) }.cast_mut() + } + + /// Not intended to be called externally, except via declare_drm_ioctls!() + /// + /// # Safety + /// + /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count, + /// i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points + /// to can't drop to zero, for the duration of this function call and the entire duration when + /// the returned reference exists. + /// + /// Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is + /// embedded in `Self`. + #[doc(hidden)] + pub unsafe fn as_ref<'a>(ptr: *const bindings::drm_device) -> &'a Self { + // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a + // `struct drm_device` embedded in `Self`. + let ptr = unsafe { Self::from_drm_device(ptr) }; + + // SAFETY: `ptr` is valid by the safety requirements of this function. + unsafe { &*ptr.cast() } + } +} + +impl Deref for Device { + type Target = T::Data; + + fn deref(&self) -> &Self::Target { + &self.data + } +} + +// SAFETY: DRM device objects are always reference counted and the get/put functions +// satisfy the requirements. +unsafe impl AlwaysRefCounted for Device { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_dev_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::drm_dev_put(obj.cast().as_ptr()) }; + } +} + +impl AsRef for Device { + fn as_ref(&self) -> &device::Device { + // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid, + // which is guaranteed by the type invariant. + unsafe { device::Device::as_ref((*self.as_raw()).dev) } + } +} + +// SAFETY: A `drm::Device` can be released from any thread. +unsafe impl Send for Device {} + +// SAFETY: A `drm::Device` can be shared among threads because all immutable methods are protected +// by the synchronization in `struct drm_device`. +unsafe impl Sync for Device {} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index c85cf5e4cd7c..26a5d66cc4c5 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -8,7 +8,6 @@ use crate::{bindings, drm, str::CStr}; use macros::vtable; /// Driver use the GEM memory manager. This should be set for all modern drivers. -#[expect(unused)] pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM; /// Information data for a DRM Driver. @@ -27,14 +26,12 @@ pub struct DriverInfo { /// Internal memory management operation set, normally created by memory managers (e.g. GEM). pub struct AllocOps { - #[expect(unused)] pub(crate) gem_create_object: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, size: usize, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) prime_handle_to_fd: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -44,7 +41,6 @@ pub struct AllocOps { prime_fd: *mut core::ffi::c_int, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) prime_fd_to_handle: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -53,14 +49,12 @@ pub struct AllocOps { handle: *mut u32, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) gem_prime_import: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, dma_buf: *mut bindings::dma_buf, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) gem_prime_import_sg_table: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -68,7 +62,6 @@ pub struct AllocOps { sgt: *mut bindings::sg_table, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) dumb_create: Option< unsafe extern "C" fn( file_priv: *mut bindings::drm_file, @@ -76,7 +69,6 @@ pub struct AllocOps { args: *mut bindings::drm_mode_create_dumb, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) dumb_map_offset: Option< unsafe extern "C" fn( file_priv: *mut bindings::drm_file, diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 2e3f9a8a9353..967854a2083e 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -2,9 +2,11 @@ //! DRM subsystem abstractions. +pub mod device; pub mod driver; pub mod ioctl; +pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; -- cgit From 0600032c54b7adc309d334c109374433ce3ab243 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:24 +0200 Subject: rust: drm: add DRM driver registration Implement the DRM driver `Registration`. The `Registration` structure is responsible to register and unregister a DRM driver. It makes use of the `Devres` container in order to allow the `Registration` to be owned by devres, such that it is automatically dropped (and the DRM driver unregistered) once the parent device is unbound. Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-6-dakr@kernel.org [ Rework of drm::Registration * move VTABLE to drm::Device to prevent use-after-free bugs; VTABLE needs to be bound to the lifetime of drm::Device, not the drm::Registration * combine new() and register() to get rid of the registered boolean * remove file_operations * move struct drm_device creation to drm::Device * introduce Devres * original source archive: https://archive.is/Pl9ys - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/driver.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++- rust/kernel/drm/mod.rs | 1 + 2 files changed, 60 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 26a5d66cc4c5..4f80bc448c09 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -4,7 +4,15 @@ //! //! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h) -use crate::{bindings, drm, str::CStr}; +use crate::{ + bindings, device, + devres::Devres, + drm, + error::{to_result, Result}, + prelude::*, + str::CStr, + types::ARef, +}; use macros::vtable; /// Driver use the GEM memory manager. This should be set for all modern drivers. @@ -103,3 +111,53 @@ pub trait Driver { /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`. const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor]; } + +/// The registration type of a `drm::Device`. +/// +/// Once the `Registration` structure is dropped, the device is unregistered. +pub struct Registration(ARef>); + +impl Registration { + /// Creates a new [`Registration`] and registers it. + fn new(drm: &drm::Device, flags: usize) -> Result { + // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`. + to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?; + + Ok(Self(drm.into())) + } + + /// Same as [`Registration::new`}, but transfers ownership of the [`Registration`] to + /// [`Devres`]. + pub fn new_foreign_owned( + drm: &drm::Device, + dev: &device::Device, + flags: usize, + ) -> Result { + if drm.as_ref().as_raw() != dev.as_raw() { + return Err(EINVAL); + } + + let reg = Registration::::new(drm, flags)?; + Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + } + + /// Returns a reference to the `Device` instance for this registration. + pub fn device(&self) -> &drm::Device { + &self.0 + } +} + +// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between +// threads, hence it's safe to share it. +unsafe impl Sync for Registration {} + +// SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread. +unsafe impl Send for Registration {} + +impl Drop for Registration { + fn drop(&mut self) { + // SAFETY: Safe by the invariant of `ARef>`. The existence of this + // `Registration` also guarantees the this `drm::Device` is actually registered. + unsafe { bindings::drm_dev_unregister(self.0.as_raw()) }; + } +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 967854a2083e..2d88e70ba607 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -9,6 +9,7 @@ pub mod ioctl; pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; +pub use self::driver::Registration; pub(crate) mod private { pub trait Sealed {} -- cgit From a98a73be9ee9c42495690b2fe56e1ce27768289a Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:25 +0200 Subject: rust: drm: file: Add File abstraction A DRM File is the DRM counterpart to a kernel file structure, representing an open DRM file descriptor. Add a Rust abstraction to allow drivers to implement their own File types that implement the DriverFile trait. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-7-dakr@kernel.org [ Rework of drm::File * switch to the Opaque type * fix (mutable) references to struct drm_file (which in this context is UB) * restructure and rename functions to align with common kernel schemes * write and fix safety and invariant comments * remove necessity for and convert 'as' casts * original source archive: https://archive.is/GH8oy - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 4 +- rust/kernel/drm/driver.rs | 3 ++ rust/kernel/drm/file.rs | 99 +++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 2 + 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 rust/kernel/drm/file.rs (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index d6c6cecbdd51..520f51bb40af 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -63,8 +63,8 @@ pub struct Device { impl Device { const VTABLE: bindings::drm_driver = drm_legacy_fields! { load: None, - open: None, // TODO: File abstraction - postclose: None, // TODO: File abstraction + open: Some(drm::File::::open_callback), + postclose: Some(drm::File::::postclose_callback), unload: None, release: None, master_set: None, diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 4f80bc448c09..f0c2936c3374 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -105,6 +105,9 @@ pub trait Driver { /// The type used to manage memory for this driver. type Object: AllocImpl; + /// The type used to represent a DRM File (client) + type File: drm::file::DriverFile; + /// Driver metadata const INFO: DriverInfo; diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs new file mode 100644 index 000000000000..b9527705e551 --- /dev/null +++ b/rust/kernel/drm/file.rs @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM File objects. +//! +//! C header: [`include/linux/drm/drm_file.h`](srctree/include/linux/drm/drm_file.h) + +use crate::{bindings, drm, error::Result, prelude::*, types::Opaque}; +use core::marker::PhantomData; +use core::pin::Pin; + +/// Trait that must be implemented by DRM drivers to represent a DRM File (a client instance). +pub trait DriverFile { + /// The parent `Driver` implementation for this `DriverFile`. + type Driver: drm::Driver; + + /// Open a new file (called when a client opens the DRM device). + fn open(device: &drm::Device) -> Result>>; +} + +/// An open DRM File. +/// +/// # Invariants +/// +/// `self.0` is a valid instance of a `struct drm_file`. +#[repr(transparent)] +pub struct File(Opaque, PhantomData); + +impl File { + #[doc(hidden)] + /// Not intended to be called externally, except via declare_drm_ioctls!() + /// + /// # Safety + /// + /// `raw_file` must be a valid pointer to an open `struct drm_file`, opened through `T::open`. + pub unsafe fn as_ref<'a>(ptr: *mut bindings::drm_file) -> &'a File { + // SAFETY: `raw_file` is valid by the safety requirements of this function. + unsafe { &*ptr.cast() } + } + + pub(super) fn as_raw(&self) -> *mut bindings::drm_file { + self.0.get() + } + + fn driver_priv(&self) -> *mut T { + // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid. + unsafe { (*self.as_raw()).driver_priv }.cast() + } + + /// Return a pinned reference to the driver file structure. + pub fn inner(&self) -> Pin<&T> { + // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened + // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`. + unsafe { Pin::new_unchecked(&*(self.driver_priv())) } + } + + /// The open callback of a `struct drm_file`. + pub(crate) extern "C" fn open_callback( + raw_dev: *mut bindings::drm_device, + raw_file: *mut bindings::drm_file, + ) -> core::ffi::c_int { + // SAFETY: A callback from `struct drm_driver::open` guarantees that + // - `raw_dev` is valid pointer to a `struct drm_device`, + // - the corresponding `struct drm_device` has been registered. + let drm = unsafe { drm::Device::as_ref(raw_dev) }; + + // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`. + let file = unsafe { File::::as_ref(raw_file) }; + + let inner = match T::open(drm) { + Err(e) => { + return e.to_errno(); + } + Ok(i) => i, + }; + + // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in + // `postclose_callback()`. + let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) }); + + // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid. + unsafe { (*file.as_raw()).driver_priv = driver_priv.cast() }; + + 0 + } + + /// The postclose callback of a `struct drm_file`. + pub(crate) extern "C" fn postclose_callback( + _raw_dev: *mut bindings::drm_device, + raw_file: *mut bindings::drm_file, + ) { + // SAFETY: This reference won't escape this function + let file = unsafe { File::::as_ref(raw_file) }; + + // SAFETY: `file.driver_priv` has been created in `open_callback` through `KBox::into_raw`. + let _ = unsafe { KBox::from_raw(file.driver_priv()) }; + } +} + +impl super::private::Sealed for File {} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 2d88e70ba607..b36223e5bd98 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -4,12 +4,14 @@ pub mod device; pub mod driver; +pub mod file; pub mod ioctl; pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; pub use self::driver::Registration; +pub use self::file::File; pub(crate) mod private { pub trait Sealed {} -- cgit From c284d3e423382be3591d5b1e402e330e6c3f726c Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:26 +0200 Subject: rust: drm: gem: Add GEM object abstraction DRM GEM is the DRM memory management subsystem used by most modern drivers; add a Rust abstraction for DRM GEM. This includes the BaseObject trait, which contains operations shared by all GEM object classes. Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-8-dakr@kernel.org [ Rework of GEM object abstractions * switch to the Opaque type * fix (mutable) references to struct drm_gem_object (which in this context is UB) * drop all custom reference types in favor of AlwaysRefCounted * bunch of minor changes and simplifications (e.g. IntoGEMObject trait) * write and fix safety and invariant comments * remove necessity for and convert 'as' casts * original source archive: https://archive.is/dD5SL - Danilo ] [ Fix missing CONFIG_DRM guards in rust/helpers/drm.c. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 4 +- rust/kernel/drm/driver.rs | 2 +- rust/kernel/drm/gem/mod.rs | 320 +++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 1 + 4 files changed, 325 insertions(+), 2 deletions(-) create mode 100644 rust/kernel/drm/gem/mod.rs (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 520f51bb40af..74c9a3dd719e 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -89,9 +89,11 @@ impl Device { driver_features: drm::driver::FEAT_GEM, ioctls: T::IOCTLS.as_ptr(), num_ioctls: T::IOCTLS.len() as i32, - fops: core::ptr::null_mut() as _, + fops: &Self::GEM_FOPS as _, }; + const GEM_FOPS: bindings::file_operations = drm::gem::create_fops(); + /// Create a new `drm::Device` for a `drm::Driver`. pub fn new(dev: &device::Device, data: impl PinInit) -> Result> { // SAFETY: diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index f0c2936c3374..acb638086131 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -88,7 +88,7 @@ pub struct AllocOps { } /// Trait for memory manager implementations. Implemented internally. -pub trait AllocImpl: super::private::Sealed { +pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject { /// The C callback operations for this memory manager. const ALLOC_OPS: AllocOps; } diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs new file mode 100644 index 000000000000..0cafa4a42420 --- /dev/null +++ b/rust/kernel/drm/gem/mod.rs @@ -0,0 +1,320 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM GEM API +//! +//! C header: [`include/linux/drm/drm_gem.h`](srctree/include/linux/drm/drm_gem.h) + +use crate::{ + alloc::flags::*, + bindings, drm, + drm::driver::{AllocImpl, AllocOps}, + error::{to_result, Result}, + prelude::*, + types::{ARef, Opaque}, +}; +use core::{mem, ops::Deref, ptr, ptr::NonNull}; + +/// GEM object functions, which must be implemented by drivers. +pub trait BaseDriverObject: Sync + Send + Sized { + /// Create a new driver data object for a GEM object of a given size. + fn new(dev: &drm::Device, size: usize) -> impl PinInit; + + /// Open a new handle to an existing object, associated with a File. + fn open( + _obj: &<::Driver as drm::Driver>::Object, + _file: &drm::File<<::Driver as drm::Driver>::File>, + ) -> Result { + Ok(()) + } + + /// Close a handle to an existing object, associated with a File. + fn close( + _obj: &<::Driver as drm::Driver>::Object, + _file: &drm::File<<::Driver as drm::Driver>::File>, + ) { + } +} + +/// Trait that represents a GEM object subtype +pub trait IntoGEMObject: Sized + super::private::Sealed { + /// Owning driver for this type + type Driver: drm::Driver; + + /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as + /// this owning object is valid. + #[allow(clippy::wrong_self_convention)] + fn into_gem_obj(&self) -> &Opaque; + + /// Converts a pointer to a `struct drm_gem_object` into a pointer to `Self`. + fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self; +} + +/// Trait which must be implemented by drivers using base GEM objects. +pub trait DriverObject: BaseDriverObject> { + /// Parent `Driver` for this object. + type Driver: drm::Driver; +} + +extern "C" fn open_callback, U: BaseObject>( + raw_obj: *mut bindings::drm_gem_object, + raw_file: *mut bindings::drm_file, +) -> core::ffi::c_int { + // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. + let file = unsafe { + drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) + }; + let obj = + <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( + raw_obj, + ); + + // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the + // `raw_obj` we got is valid. + match T::open(unsafe { &*obj }, file) { + Err(e) => e.to_errno(), + Ok(()) => 0, + } +} + +extern "C" fn close_callback, U: BaseObject>( + raw_obj: *mut bindings::drm_gem_object, + raw_file: *mut bindings::drm_file, +) { + // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. + let file = unsafe { + drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) + }; + let obj = + <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( + raw_obj, + ); + + // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the + // `raw_obj` we got is valid. + T::close(unsafe { &*obj }, file); +} + +impl IntoGEMObject for Object { + type Driver = T::Driver; + + fn into_gem_obj(&self) -> &Opaque { + &self.obj + } + + fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self { + // SAFETY: All of our objects are Object. + unsafe { crate::container_of!(obj, Object, obj).cast_mut() } + } +} + +/// Base operations shared by all GEM object classes +pub trait BaseObject +where + Self: crate::types::AlwaysRefCounted + IntoGEMObject, +{ + /// Returns the size of the object in bytes. + fn size(&self) -> usize { + // SAFETY: `self.into_gem_obj()` is guaranteed to be a pointer to a valid `struct + // drm_gem_object`. + unsafe { (*self.into_gem_obj().get()).size } + } + + /// Creates a new handle for the object associated with a given `File` + /// (or returns an existing one). + fn create_handle( + &self, + file: &drm::File<<::Driver as drm::Driver>::File>, + ) -> Result { + let mut handle: u32 = 0; + // SAFETY: The arguments are all valid per the type invariants. + to_result(unsafe { + bindings::drm_gem_handle_create( + file.as_raw().cast(), + self.into_gem_obj().get(), + &mut handle, + ) + })?; + Ok(handle) + } + + /// Looks up an object by its handle for a given `File`. + fn lookup_handle( + file: &drm::File<<::Driver as drm::Driver>::File>, + handle: u32, + ) -> Result> { + // SAFETY: The arguments are all valid per the type invariants. + let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; + let ptr = ::from_gem_obj(ptr); + let ptr = NonNull::new(ptr).ok_or(ENOENT)?; + + // SAFETY: We take ownership of the reference of `drm_gem_object_lookup()`. + Ok(unsafe { ARef::from_raw(ptr) }) + } + + /// Creates an mmap offset to map the object from userspace. + fn create_mmap_offset(&self) -> Result { + // SAFETY: The arguments are valid per the type invariant. + to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.into_gem_obj().get()) })?; + + // SAFETY: The arguments are valid per the type invariant. + Ok(unsafe { + bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!( + (*self.into_gem_obj().get()).vma_node + )) + }) + } +} + +impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObject {} + +/// A base GEM object. +/// +/// Invariants +/// +/// - `self.obj` is a valid instance of a `struct drm_gem_object`. +/// - `self.dev` is always a valid pointer to a `struct drm_device`. +#[repr(C)] +#[pin_data] +pub struct Object { + obj: Opaque, + dev: *const drm::Device, + #[pin] + data: T, +} + +impl Object { + /// The size of this object's structure. + pub const SIZE: usize = mem::size_of::(); + + const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { + free: Some(Self::free_callback), + open: Some(open_callback::>), + close: Some(close_callback::>), + print_info: None, + export: None, + pin: None, + unpin: None, + get_sg_table: None, + vmap: None, + vunmap: None, + mmap: None, + status: None, + vm_ops: core::ptr::null_mut(), + evict: None, + rss: None, + }; + + /// Create a new GEM object. + pub fn new(dev: &drm::Device, size: usize) -> Result> { + let obj: Pin> = KBox::pin_init( + try_pin_init!(Self { + obj: Opaque::new(bindings::drm_gem_object::default()), + data <- T::new(dev, size), + // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live + // as long as the GEM object lives. + dev, + }), + GFP_KERNEL, + )?; + + // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above. + unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS }; + + // SAFETY: The arguments are all valid per the type invariants. + to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj.obj.get(), size) })?; + + // SAFETY: We never move out of `Self`. + let ptr = KBox::into_raw(unsafe { Pin::into_inner_unchecked(obj) }); + + // SAFETY: `ptr` comes from `KBox::into_raw` and hence can't be NULL. + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + + // SAFETY: We take over the initial reference count from `drm_gem_object_init()`. + Ok(unsafe { ARef::from_raw(ptr) }) + } + + /// Returns the `Device` that owns this GEM object. + pub fn dev(&self) -> &drm::Device { + // SAFETY: The DRM subsystem guarantees that the `struct drm_device` will live as long as + // the GEM object lives, hence the pointer must be valid. + unsafe { &*self.dev } + } + + fn as_raw(&self) -> *mut bindings::drm_gem_object { + self.obj.get() + } + + extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { + // SAFETY: All of our objects are of type `Object`. + let this = unsafe { crate::container_of!(obj, Self, obj) }.cast_mut(); + + // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct + // drm_gem_object`. + unsafe { bindings::drm_gem_object_release(obj) }; + + // SAFETY: All of our objects are allocated via `KBox`, and we're in the + // free callback which guarantees this object has zero remaining references, + // so we can drop it. + let _ = unsafe { KBox::from_raw(this) }; + } +} + +// SAFETY: Instances of `Object` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Object { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: `obj` is a valid pointer to an `Object`. + let obj = unsafe { obj.as_ref() }; + + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::drm_gem_object_put(obj.as_raw()) } + } +} + +impl super::private::Sealed for Object {} + +impl Deref for Object { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.data + } +} + +impl AllocImpl for Object { + const ALLOC_OPS: AllocOps = AllocOps { + gem_create_object: None, + prime_handle_to_fd: None, + prime_fd_to_handle: None, + gem_prime_import: None, + gem_prime_import_sg_table: None, + dumb_create: None, + dumb_map_offset: None, + }; +} + +pub(super) const fn create_fops() -> bindings::file_operations { + // SAFETY: As by the type invariant, it is safe to initialize `bindings::file_operations` + // zeroed. + let mut fops: bindings::file_operations = unsafe { core::mem::zeroed() }; + + fops.owner = core::ptr::null_mut(); + fops.open = Some(bindings::drm_open); + fops.release = Some(bindings::drm_release); + fops.unlocked_ioctl = Some(bindings::drm_ioctl); + #[cfg(CONFIG_COMPAT)] + { + fops.compat_ioctl = Some(bindings::drm_compat_ioctl); + } + fops.poll = Some(bindings::drm_poll); + fops.read = Some(bindings::drm_read); + fops.llseek = Some(bindings::noop_llseek); + fops.mmap = Some(bindings::drm_gem_mmap); + fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET; + + fops +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index b36223e5bd98..1b82b6945edf 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -5,6 +5,7 @@ pub mod device; pub mod driver; pub mod file; +pub mod gem; pub mod ioctl; pub use self::device::Device; -- cgit From fc55584e00fc8409cbaef4bcd984016dca6f1b6b Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 29 Apr 2025 23:06:29 +0200 Subject: rust: device: conditionally expect `dead_code` for `parent()` When `CONFIG_AUXILIARY_BUS` is disabled, `parent()` is still dead code: error: method `parent` is never used --> rust/kernel/device.rs:71:19 | 64 | impl Device { | ------------------------------------ method in this implementation ... 71 | pub(crate) fn parent(&self) -> Option<&Self> { | ^^^^^^ | = note: `-D dead-code` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(dead_code)]` Thus reintroduce the `expect`, but now as a conditional one. Do so as `dead_code` since that is narrower. An `allow` would also be possible, but Danilo wants to catch new users in the future [1]. Link: https://lore.kernel.org/rust-for-linux/aBE8qQrpXOfru_K3@pollux/ [1] Fixes: ce735e73dd59 ("rust: auxiliary: add auxiliary device / driver abstractions") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250429210629.513521-1-ojeda@kernel.org [ Adjust commit subject to "rust: device: conditionally expect `dead_code` for `parent()`". - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 40c1f549b0ba..f08583fa39c9 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -68,6 +68,7 @@ impl Device { } /// Returns a reference to the parent device, if any. + #[cfg_attr(not(CONFIG_AUXILIARY_BUS), expect(dead_code))] pub(crate) fn parent(&self) -> Option<&Self> { // SAFETY: // - By the type invariant `self.as_raw()` is always valid. -- cgit From 46f91addfabbd4109fb64876a032ae4a4a924919 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 28 Apr 2025 16:00:27 +0200 Subject: rust: revocable: implement Revocable::access() Implement an unsafe direct accessor for the data stored within the Revocable. This is useful for cases where we can prove that the data stored within the Revocable is not and cannot be revoked for the duration of the lifetime of the returned reference. Reviewed-by: Christian Schrefl Reviewed-by: Benno Lossin Acked-by: Miguel Ojeda Reviewed-by: Alexandre Courbot Acked-by: Boqun Feng Reviewed-by: Joel Fernandes Link: https://lore.kernel.org/r/20250428140137.468709-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index 971d0dc38d83..db4aa46bb121 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -139,6 +139,18 @@ impl Revocable { self.try_access().map(|t| f(&*t)) } + /// Directly access the revocable wrapped object. + /// + /// # Safety + /// + /// The caller must ensure this [`Revocable`] instance hasn't been revoked and won't be revoked + /// as long as the returned `&T` lives. + pub unsafe fn access(&self) -> &T { + // SAFETY: By the safety requirement of this function it is guaranteed that + // `self.data.get()` is a valid pointer to an instance of `T`. + unsafe { &*self.data.get() } + } + /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. -- cgit From f301cb978c068faa8fcd630be2cb317a2d0ec063 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 28 Apr 2025 16:00:28 +0200 Subject: rust: devres: implement Devres::access() Implement a direct accessor for the data stored within the Devres for cases where we can prove that we own a reference to a Device (i.e. a bound device) of the same device that was used to create the corresponding Devres container. Usually, when accessing the data stored within a Devres container, it is not clear whether the data has been revoked already due to the device being unbound and, hence, we have to try whether the access is possible and subsequently keep holding the RCU read lock for the duration of the access. However, when we can prove that we hold a reference to Device matching the device the Devres container has been created with, we can guarantee that the device is not unbound for the duration of the lifetime of the Device reference and, hence, it is not possible for the data within the Devres container to be revoked. Therefore, in this case, we can bypass the atomic check and the RCU read lock, which is a great optimization and simplification for drivers. Reviewed-by: Christian Schrefl Reviewed-by: Alexandre Courbot Acked-by: Boqun Feng Reviewed-by: Joel Fernandes Link: https://lore.kernel.org/r/20250428140137.468709-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 1e58f5d22044..b67247db92c7 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -181,6 +181,44 @@ impl Devres { Ok(()) } + + /// Obtain `&'a T`, bypassing the [`Revocable`]. + /// + /// This method allows to directly obtain a `&'a T`, bypassing the [`Revocable`], by presenting + /// a `&'a Device` of the same [`Device`] this [`Devres`] instance has been created with. + /// + /// # Errors + /// + /// An error is returned if `dev` does not match the same [`Device`] this [`Devres`] instance + /// has been created with. + /// + /// # Example + /// + /// ```no_run + /// # use kernel::{device::Core, devres::Devres, pci}; + /// + /// fn from_core(dev: &pci::Device, devres: Devres>) -> Result { + /// let bar = devres.access(dev.as_ref())?; + /// + /// let _ = bar.read32(0x0); + /// + /// // might_sleep() + /// + /// bar.write32(0x42, 0x0); + /// + /// Ok(()) + /// } + /// ``` + pub fn access<'a>(&'a self, dev: &'a Device) -> Result<&'a T> { + if self.0.dev.as_raw() != dev.as_raw() { + return Err(EINVAL); + } + + // SAFETY: `dev` being the same device as the device this `Devres` has been created for + // proves that `self.0.data` hasn't been revoked and is guaranteed to not be revoked as + // long as `dev` lives; `dev` lives at least as long as `self`. + Ok(unsafe { self.deref().access() }) + } } impl Deref for Devres { -- cgit From 42055939a3a4cac8afbebff85a29571c2bd3238c Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 11 May 2025 20:25:33 +0200 Subject: rust: devres: fix doctest build under `!CONFIG_PCI` The doctest requires `CONFIG_PCI`: error[E0432]: unresolved import `kernel::pci` --> rust/doctests_kernel_generated.rs:2689:44 | 2689 | use kernel::{device::Core, devres::Devres, pci}; | ^^^ no `pci` in the root | note: found an item that was configured out --> rust/kernel/lib.rs:96:9 note: the item is gated here --> rust/kernel/lib.rs:95:1 Thus conditionally compile it (which still checks the syntax). Fixes: f301cb978c06 ("rust: devres: implement Devres::access()") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250511182533.1016163-1-ojeda@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index b67247db92c7..0f79a2ec9474 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -195,6 +195,7 @@ impl Devres { /// # Example /// /// ```no_run + /// # #![cfg(CONFIG_PCI)] /// # use kernel::{device::Core, devres::Devres, pci}; /// /// fn from_core(dev: &pci::Device, devres: Devres>) -> Result { -- cgit From 6ee48aee8c705717051109ff889f2a1d4f409ea9 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:54 -0400 Subject: rust: drm: gem: Use NonNull for Object::dev There is usually not much of a reason to use a raw pointer in a data struct, so move this to NonNull instead. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-2-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 0cafa4a42420..df8f9fdae5c2 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -177,7 +177,7 @@ impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObj #[pin_data] pub struct Object { obj: Opaque, - dev: *const drm::Device, + dev: NonNull>, #[pin] data: T, } @@ -212,7 +212,7 @@ impl Object { data <- T::new(dev, size), // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live // as long as the GEM object lives. - dev, + dev: dev.into(), }), GFP_KERNEL, )?; @@ -237,7 +237,7 @@ impl Object { pub fn dev(&self) -> &drm::Device { // SAFETY: The DRM subsystem guarantees that the `struct drm_device` will live as long as // the GEM object lives, hence the pointer must be valid. - unsafe { &*self.dev } + unsafe { self.dev.as_ref() } } fn as_raw(&self) -> *mut bindings::drm_gem_object { -- cgit From 36b1ccbfa0c2f45fae3e07bf66e40d7fd6704874 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:55 -0400 Subject: rust: drm: gem: Refactor IntoGEMObject::from_gem_obj() to as_ref() There's a few issues with this function, mainly: * This function -probably- should have been unsafe from the start. Pointers are not always necessarily valid, but you want a function that does field-projection for a pointer that can travel outside of the original struct to be unsafe, at least if I understand properly. * *mut Self is not terribly useful in this context, the majority of uses of from_gem_obj() grab a *mut Self and then immediately convert it into a &'a Self. It also goes against the ffi conventions we've set in the rest of the kernel thus far. * from_gem_obj() also doesn't follow the naming conventions in the rest of the DRM bindings at the moment, as_ref() would be a better name. So, let's: * Make from_gem_obj() unsafe * Convert it to return &'a Self * Rename it to as_ref() * Update all call locations Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-3-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 69 +++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 26 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index df8f9fdae5c2..1ea1f15d8313 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -45,8 +45,14 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { #[allow(clippy::wrong_self_convention)] fn into_gem_obj(&self) -> &Opaque; - /// Converts a pointer to a `struct drm_gem_object` into a pointer to `Self`. - fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self; + /// Converts a pointer to a `struct drm_gem_object` into a reference to `Self`. + /// + /// # Safety + /// + /// - `self_ptr` must be a valid pointer to `Self`. + /// - The caller promises that holding the immutable reference returned by this function does + /// not violate rust's data aliasing rules and remains valid throughout the lifetime of `'a`. + unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self; } /// Trait which must be implemented by drivers using base GEM objects. @@ -63,14 +69,13 @@ extern "C" fn open_callback, U: BaseObject>( let file = unsafe { drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) }; - let obj = - <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( - raw_obj, - ); - - // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the - // `raw_obj` we got is valid. - match T::open(unsafe { &*obj }, file) { + // SAFETY: `open_callback` is specified in the AllocOps structure for `Object`, ensuring that + // `raw_obj` is indeed contained within a `Object`. + let obj = unsafe { + <<::Driver as drm::Driver>::Object as IntoGEMObject>::as_ref(raw_obj) + }; + + match T::open(obj, file) { Err(e) => e.to_errno(), Ok(()) => 0, } @@ -84,14 +89,13 @@ extern "C" fn close_callback, U: BaseObject>( let file = unsafe { drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) }; - let obj = - <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( - raw_obj, - ); - - // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the - // `raw_obj` we got is valid. - T::close(unsafe { &*obj }, file); + // SAFETY: `close_callback` is specified in the AllocOps structure for `Object`, ensuring + // that `raw_obj` is indeed contained within a `Object`. + let obj = unsafe { + <<::Driver as drm::Driver>::Object as IntoGEMObject>::as_ref(raw_obj) + }; + + T::close(obj, file); } impl IntoGEMObject for Object { @@ -101,9 +105,10 @@ impl IntoGEMObject for Object { &self.obj } - fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self { - // SAFETY: All of our objects are Object. - unsafe { crate::container_of!(obj, Object, obj).cast_mut() } + unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { + // SAFETY: `obj` is guaranteed to be in an `Object` via the safety contract of this + // function + unsafe { &*crate::container_of!(self_ptr, Object, obj) } } } @@ -144,11 +149,23 @@ where ) -> Result> { // SAFETY: The arguments are all valid per the type invariants. let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; - let ptr = ::from_gem_obj(ptr); - let ptr = NonNull::new(ptr).ok_or(ENOENT)?; - - // SAFETY: We take ownership of the reference of `drm_gem_object_lookup()`. - Ok(unsafe { ARef::from_raw(ptr) }) + if ptr.is_null() { + return Err(ENOENT); + } + + // SAFETY: + // - A `drm::Driver` can only have a single `File` implementation. + // - `file` uses the same `drm::Driver` as `Self`. + // - Therefore, we're guaranteed that `ptr` must be a gem object embedded within `Self`. + // - And we check if the pointer is null befoe calling as_ref(), ensuring that `ptr` is a + // valid pointer to an initialized `Self`. + let obj = unsafe { Self::as_ref(ptr) }; + + // SAFETY: + // - We take ownership of the reference of `drm_gem_object_lookup()`. + // - Our `NonNull` comes from an immutable reference, thus ensuring it is a valid pointer to + // `Self`. + Ok(unsafe { ARef::from_raw(obj.into()) }) } /// Creates an mmap offset to map the object from userspace. -- cgit From b36ff40b4abd0f468564cfe9805f0d8e850cafe9 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:56 -0400 Subject: rust: drm: gem: s/into_gem_obj()/as_raw()/ There's a few changes here: * The rename, of course (this should also let us drop the clippy annotation here) * Return *mut bindings::drm_gem_object instead of &Opaque - the latter doesn't really have any benefit and just results in conversion from the rust type to the C type having to be more verbose than necessary. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-4-lyude@redhat.com [ Fixup s/into_gem_obj()/as_raw()/ in safety comment. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 1ea1f15d8313..e11cf7837bf5 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -12,7 +12,7 @@ use crate::{ prelude::*, types::{ARef, Opaque}, }; -use core::{mem, ops::Deref, ptr, ptr::NonNull}; +use core::{mem, ops::Deref, ptr::NonNull}; /// GEM object functions, which must be implemented by drivers. pub trait BaseDriverObject: Sync + Send + Sized { @@ -42,8 +42,7 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as /// this owning object is valid. - #[allow(clippy::wrong_self_convention)] - fn into_gem_obj(&self) -> &Opaque; + fn as_raw(&self) -> *mut bindings::drm_gem_object; /// Converts a pointer to a `struct drm_gem_object` into a reference to `Self`. /// @@ -101,8 +100,8 @@ extern "C" fn close_callback, U: BaseObject>( impl IntoGEMObject for Object { type Driver = T::Driver; - fn into_gem_obj(&self) -> &Opaque { - &self.obj + fn as_raw(&self) -> *mut bindings::drm_gem_object { + self.obj.get() } unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { @@ -119,9 +118,8 @@ where { /// Returns the size of the object in bytes. fn size(&self) -> usize { - // SAFETY: `self.into_gem_obj()` is guaranteed to be a pointer to a valid `struct - // drm_gem_object`. - unsafe { (*self.into_gem_obj().get()).size } + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `struct drm_gem_object`. + unsafe { (*self.as_raw()).size } } /// Creates a new handle for the object associated with a given `File` @@ -133,11 +131,7 @@ where let mut handle: u32 = 0; // SAFETY: The arguments are all valid per the type invariants. to_result(unsafe { - bindings::drm_gem_handle_create( - file.as_raw().cast(), - self.into_gem_obj().get(), - &mut handle, - ) + bindings::drm_gem_handle_create(file.as_raw().cast(), self.as_raw(), &mut handle) })?; Ok(handle) } @@ -171,14 +165,10 @@ where /// Creates an mmap offset to map the object from userspace. fn create_mmap_offset(&self) -> Result { // SAFETY: The arguments are valid per the type invariant. - to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.into_gem_obj().get()) })?; + to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.as_raw()) })?; // SAFETY: The arguments are valid per the type invariant. - Ok(unsafe { - bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!( - (*self.into_gem_obj().get()).vma_node - )) - }) + Ok(unsafe { bindings::drm_vma_node_offset_addr(&raw mut (*self.as_raw()).vma_node) }) } } -- cgit From 38cb08c3fcd3f3b1d0225dcec8ae50fab5751549 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:57 -0400 Subject: rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically Currently we are requiring AlwaysRefCounted in most trait bounds for gem objects, and implementing it by hand for our only current type of gem object. However, all gem objects use the same functions for reference counting - and all gem objects support reference counting. We're planning on adding support for shmem gem objects, let's move this around a bit by instead making IntoGEMObject require AlwaysRefCounted as a trait bound, and then provide a blanket AlwaysRefCounted implementation for any object that implements IntoGEMObject so all gem object types can use the same AlwaysRefCounted implementation. This also makes things less verbose by making the AlwaysRefCounted trait bound implicit for any IntoGEMObject bound. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-5-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 47 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index e11cf7837bf5..d8765e61c6c2 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,7 +10,7 @@ use crate::{ drm::driver::{AllocImpl, AllocOps}, error::{to_result, Result}, prelude::*, - types::{ARef, Opaque}, + types::{ARef, AlwaysRefCounted, Opaque}, }; use core::{mem, ops::Deref, ptr::NonNull}; @@ -36,7 +36,7 @@ pub trait BaseDriverObject: Sync + Send + Sized { } /// Trait that represents a GEM object subtype -pub trait IntoGEMObject: Sized + super::private::Sealed { +pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { /// Owning driver for this type type Driver: drm::Driver; @@ -54,6 +54,26 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self; } +// SAFETY: All gem objects are refcounted. +unsafe impl AlwaysRefCounted for T { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: We either hold the only refcount on `obj`, or one of many - meaning that no one + // else could possibly hold a mutable reference to `obj` and thus this immutable reference + // is safe. + let obj = unsafe { obj.as_ref() }.as_raw(); + + // SAFETY: + // - The safety requirements guarantee that the refcount is non-zero. + // - We hold no references to `obj` now, making it safe for us to potentially deallocate it. + unsafe { bindings::drm_gem_object_put(obj) }; + } +} + /// Trait which must be implemented by drivers using base GEM objects. pub trait DriverObject: BaseDriverObject> { /// Parent `Driver` for this object. @@ -112,10 +132,7 @@ impl IntoGEMObject for Object { } /// Base operations shared by all GEM object classes -pub trait BaseObject -where - Self: crate::types::AlwaysRefCounted + IntoGEMObject, -{ +pub trait BaseObject: IntoGEMObject { /// Returns the size of the object in bytes. fn size(&self) -> usize { // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `struct drm_gem_object`. @@ -172,7 +189,7 @@ where } } -impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObject {} +impl BaseObject for T {} /// A base GEM object. /// @@ -266,22 +283,6 @@ impl Object { } } -// SAFETY: Instances of `Object` are always reference-counted. -unsafe impl crate::types::AlwaysRefCounted for Object { - fn inc_ref(&self) { - // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::drm_gem_object_get(self.as_raw()) }; - } - - unsafe fn dec_ref(obj: NonNull) { - // SAFETY: `obj` is a valid pointer to an `Object`. - let obj = unsafe { obj.as_ref() }; - - // SAFETY: The safety requirements guarantee that the refcount is non-zero. - unsafe { bindings::drm_gem_object_put(obj.as_raw()) } - } -} - impl super::private::Sealed for Object {} impl Deref for Object { -- cgit